ipn/ipnlocal: fix data race when accessing b.appConnector

The field must only be accessed while holding LocalBackend's mutex,
but there are two places where it's accessed without the mutex:
 - (LocalBackend).MaybeClearAppConnector()
 - handleC2NAppConnectorDomainRoutesGet()

Fixes #16123

Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
Nick Khyl 2025-05-29 13:51:46 -05:00 committed by Nick Khyl
parent dca4036a20
commit 4cccd15eeb
2 changed files with 15 additions and 7 deletions

View File

@ -240,13 +240,14 @@ func handleC2NAppConnectorDomainRoutesGet(b *LocalBackend, w http.ResponseWriter
b.logf("c2n: GET /appconnector/routes received")
var res tailcfg.C2NAppConnectorDomainRoutesResponse
if b.appConnector == nil {
appConnector := b.AppConnector()
if appConnector == nil {
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(res)
return
}
res.Domains = b.appConnector.DomainRoutes()
res.Domains = appConnector.DomainRoutes()
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(res)

View File

@ -4150,8 +4150,8 @@ func (b *LocalBackend) SetUseExitNodeEnabled(v bool) (ipn.PrefsView, error) {
// AdvertiseRoutes has been set in the MaskedPrefs.
func (b *LocalBackend) MaybeClearAppConnector(mp *ipn.MaskedPrefs) error {
var err error
if b.appConnector != nil && mp.AdvertiseRoutesSet {
err = b.appConnector.ClearRoutes()
if ac := b.AppConnector(); ac != nil && mp.AdvertiseRoutesSet {
err = ac.ClearRoutes()
if err != nil {
b.logf("appc: clear routes error: %v", err)
}
@ -4755,9 +4755,7 @@ func (b *LocalBackend) readvertiseAppConnectorRoutes() {
//
// Grab a copy of the field, since b.mu only guards access to the
// b.appConnector field itself.
b.mu.Lock()
appConnector := b.appConnector
b.mu.Unlock()
appConnector := b.AppConnector()
if appConnector == nil {
return
@ -6432,6 +6430,15 @@ func (b *LocalBackend) OfferingAppConnector() bool {
return b.appConnector != nil
}
// AppConnector returns the current AppConnector, or nil if not configured.
//
// TODO(nickkhyl): move app connectors to [nodeBackend], or perhaps a feature package?
func (b *LocalBackend) AppConnector() *appc.AppConnector {
b.mu.Lock()
defer b.mu.Unlock()
return b.appConnector
}
// allowExitNodeDNSProxyToServeName reports whether the Exit Node DNS
// proxy is allowed to serve responses for the provided DNS name.
func (b *LocalBackend) allowExitNodeDNSProxyToServeName(name string) bool {