From f52273767fd326470c362a828e63dc1907436860 Mon Sep 17 00:00:00 2001
From: Maisem Ali <maisem@tailscale.com>
Date: Thu, 31 Aug 2023 17:55:47 -0700
Subject: [PATCH] ipn/ipnlocal: fix missing mutex usage for profileManager

It required holding b.mu but was documented incorrectly, fix.

Updates #cleanup

Signed-off-by: Maisem Ali <maisem@tailscale.com>
---
 ipn/ipnlocal/local.go    | 15 ++++++++-------
 ipn/ipnlocal/profiles.go |  2 ++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index b5c949def..550b15dc0 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -143,9 +143,8 @@ type LocalBackend struct {
 	statsLogf             logger.Logf        // for printing peers stats on change
 	sys                   *tsd.System
 	e                     wgengine.Engine // non-nil; TODO(bradfitz): remove; use sys
-	pm                    *profileManager
-	store                 ipn.StateStore // non-nil; TODO(bradfitz): remove; use sys
-	dialer                *tsdial.Dialer // non-nil; TODO(bradfitz): remove; use sys
+	store                 ipn.StateStore  // non-nil; TODO(bradfitz): remove; use sys
+	dialer                *tsdial.Dialer  // non-nil; TODO(bradfitz): remove; use sys
 	backendLogID          logid.PublicID
 	unregisterNetMon      func()
 	unregisterHealthWatch func()
@@ -188,6 +187,7 @@ type LocalBackend struct {
 
 	// The mutex protects the following elements.
 	mu             sync.Mutex
+	pm             *profileManager // mu guards access
 	filterHash     deephash.Sum
 	httpTestClient *http.Client // for controlclient. nil by default, used by tests.
 	ccGen          clientGen    // function for producing controlclient; lazily populated
@@ -1289,10 +1289,6 @@ func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
 // actually a supported operation (it should be, but it's very unclear
 // from the following whether or not that is a safe transition).
 func (b *LocalBackend) Start(opts ipn.Options) error {
-	if opts.LegacyMigrationPrefs == nil && !b.pm.CurrentPrefs().Valid() {
-		return errors.New("no prefs provided")
-	}
-
 	if opts.LegacyMigrationPrefs != nil {
 		b.logf("Start: %v", opts.LegacyMigrationPrefs.Pretty())
 	} else {
@@ -1300,6 +1296,11 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	}
 
 	b.mu.Lock()
+	if opts.LegacyMigrationPrefs == nil && !b.pm.CurrentPrefs().Valid() {
+		b.mu.Unlock()
+		return errors.New("no prefs provided")
+	}
+
 	if opts.UpdatePrefs != nil {
 		if err := b.checkPrefsLocked(opts.UpdatePrefs); err != nil {
 			b.mu.Unlock()
diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go
index 97ed0b1c9..f99f8bee8 100644
--- a/ipn/ipnlocal/profiles.go
+++ b/ipn/ipnlocal/profiles.go
@@ -28,6 +28,8 @@ var debug = envknob.RegisterBool("TS_DEBUG_PROFILES")
 
 // profileManager is a wrapper around a StateStore that manages
 // multiple profiles and the current profile.
+//
+// It is not safe for concurrent use.
 type profileManager struct {
 	store ipn.StateStore
 	logf  logger.Logf