From aed60a24323c725ed1e88fd7fcae1cdfb211a9d5 Mon Sep 17 00:00:00 2001
From: Viktor Liu <17948409+lixmal@users.noreply.github.com>
Date: Fri, 31 Jul 2026 22:53:58 +0900
Subject: [PATCH] [client] Fix daemon lock order inversion between SetConfig
 and login (#6978)

---
 client/server/lock_order_test.go | 51 ++++++++++++++++++++++++++++++++
 client/server/server.go          | 16 ++++++----
 2 files changed, 61 insertions(+), 6 deletions(-)
 create mode 100644 client/server/lock_order_test.go

diff --git a/client/server/lock_order_test.go b/client/server/lock_order_test.go
new file mode 100644
index 000000000..457e3db34
--- /dev/null
+++ b/client/server/lock_order_test.go
@@ -0,0 +1,51 @@
+package server
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/proto"
+)
+
+// The daemon takes guardedConfigMu before s.mutex. authorizeAndPrepareLogin
+// takes s.mutex while holding guardedConfigMu, so a SetConfig that grabbed
+// s.mutex first and then waited for guardedConfigMu would deadlock the daemon
+// against a concurrent login: two unprivileged IPC calls are enough.
+//
+// The held guardedConfigMu below stands in for that login. While SetConfig waits
+// for it, s.mutex must stay free, otherwise the login waiting for s.mutex could
+// never release guardedConfigMu.
+func TestSetConfig_TakesGuardedConfigMuBeforeServerMutex(t *testing.T) {
+	s, ctx, profName, username, _ := setupServerWithProfile(t)
+
+	s.guardedConfigMu.Lock()
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
+			ProfileName: profName,
+			Username:    username,
+		})
+		done <- err
+	}()
+
+	require.Never(t, func() bool {
+		if !s.mutex.TryLock() {
+			return true
+		}
+		s.mutex.Unlock()
+		return false
+	}, 500*time.Millisecond, 10*time.Millisecond,
+		"SetConfig held s.mutex while waiting for guardedConfigMu, which deadlocks against a concurrent login")
+
+	s.guardedConfigMu.Unlock()
+
+	select {
+	case err := <-done:
+		require.NoError(t, err)
+	case <-time.After(5 * time.Second):
+		t.Fatal("SetConfig did not finish after guardedConfigMu was released")
+	}
+}
diff --git a/client/server/server.go b/client/server/server.go
index db5909272..6e22a76a9 100644
--- a/client/server/server.go
+++ b/client/server/server.go
@@ -393,6 +393,16 @@ func (s *Server) loginAttempt(ctx context.Context, setupKey, jwtToken string) (i
 
 // Login uses setup key to prepare configuration for the daemon.
 func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigRequest) (*proto.SetConfigResponse, error) {
+	// Privilege gate: refuse the parts of the request that would let a local
+	// user turn the root daemon into a root shell. Held across the write so the
+	// config cannot gain the SSH server between the decision and the update.
+	//
+	// Taken before s.mutex: authorizeAndPrepareLogin takes s.mutex while holding
+	// guardedConfigMu, so acquiring the two in the other order here would let a
+	// concurrent login deadlock the daemon.
+	s.guardedConfigMu.Lock()
+	defer s.guardedConfigMu.Unlock()
+
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
 
@@ -417,12 +427,6 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques
 		return nil, err
 	}
 
-	// Privilege gate: refuse the parts of the request that would let a local
-	// user turn the root daemon into a root shell. Held across the write so the
-	// config cannot gain the SSH server between the decision and the update.
-	s.guardedConfigMu.Lock()
-	defer s.guardedConfigMu.Unlock()
-
 	stored, err := s.storedProfileConfig(msg.ProfileName, msg.Username)
 	if err != nil {
 		return nil, err