From c5c185ff71771b92c2f09d06a317695496397637 Mon Sep 17 00:00:00 2001 From: Marc Boudreau Date: Tue, 2 Jul 2024 11:03:41 -0400 Subject: [PATCH] Make mountsLock and authLock in Core configurable (#27633) * make mountsLock and authLock in Core configurable * add changelog entry --- changelog/27633.txt | 3 ++ vault/core.go | 8 +++-- vault/core_test.go | 73 +++++++++++++++++++++++++++++++++++++++++ vault/logical_system.go | 6 ++-- 4 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 changelog/27633.txt diff --git a/changelog/27633.txt b/changelog/27633.txt new file mode 100644 index 000000000000..1f5156b3bdcb --- /dev/null +++ b/changelog/27633.txt @@ -0,0 +1,3 @@ +```release-note:improvement +core: make authLock and mountsLock in Core configurable via the detect_deadlocks configuration parameter. +``` \ No newline at end of file diff --git a/vault/core.go b/vault/core.go index 30b3c284e964..4e9518cc1518 100644 --- a/vault/core.go +++ b/vault/core.go @@ -375,7 +375,7 @@ type Core struct { // mountsLock is used to ensure that the mounts table does not // change underneath a calling function - mountsLock locking.DeadlockRWMutex + mountsLock locking.RWMutex // mountMigrationTracker tracks past and ongoing remount operations // against their migration ids @@ -387,7 +387,7 @@ type Core struct { // authLock is used to ensure that the auth table does not // change underneath a calling function - authLock locking.DeadlockRWMutex + authLock locking.RWMutex // audit is loaded after unseal since it is a protected // configuration @@ -1003,6 +1003,8 @@ func CreateCore(conf *CoreConfig) (*Core, error) { detectDeadlocks := locking.ParseDetectDeadlockConfigParameter(conf.DetectDeadlocks) stateLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "statelock") + mountsLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "mountsLock") + authLock := locking.CreateConfigurableRWMutex(detectDeadlocks, "authLock") // Setup the core c := &Core{ @@ -1018,6 +1020,8 @@ func CreateCore(conf *CoreConfig) (*Core, error) { customListenerHeader: new(atomic.Value), seal: conf.Seal, stateLock: stateLock, + mountsLock: mountsLock, + authLock: authLock, router: NewRouter(), sealed: new(uint32), sealMigrationDone: new(uint32), diff --git a/vault/core_test.go b/vault/core_test.go index 8b34dd3340d7..350657405121 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -3438,6 +3438,79 @@ func InduceDeadlock(t *testing.T, vaultcore *Core, expected uint32) { } } +// TestDetectedDeadlockSetting verifies that a Core struct gets the appropriate +// locking.RWMutex implementation assigned for the stateLock, authLock, and +// mountsLock fields based on various values that could be obtained from the +// detect_deadlocks configuration parameter. +func TestDetectedDeadlockSetting(t *testing.T) { + var standardLock string = "*locking.SyncRWMutex" + var deadlockLock string = "*locking.DeadlockRWMutex" + + for _, tc := range []struct { + name string + input string + expectedDetectDeadlockSlice []string + expectedStateLockImpl string + expectedAuthLockImpl string + expectedMountsLockImpl string + }{ + { + name: "none", + input: "", + expectedDetectDeadlockSlice: []string{}, + expectedStateLockImpl: standardLock, + expectedAuthLockImpl: standardLock, + expectedMountsLockImpl: standardLock, + }, + { + name: "stateLock-only", + input: "STATELOCK", + expectedDetectDeadlockSlice: []string{"statelock"}, + expectedStateLockImpl: deadlockLock, + expectedAuthLockImpl: standardLock, + expectedMountsLockImpl: standardLock, + }, + { + name: "authLock-only", + input: "AuthLock", + expectedDetectDeadlockSlice: []string{"authlock"}, + expectedStateLockImpl: standardLock, + expectedAuthLockImpl: deadlockLock, + expectedMountsLockImpl: standardLock, + }, + { + name: "state-auth-mounts", + input: "mountsLock,AUTHlock,sTaTeLoCk", + expectedDetectDeadlockSlice: []string{"mountslock", "authlock", "statelock"}, + expectedStateLockImpl: deadlockLock, + expectedAuthLockImpl: deadlockLock, + expectedMountsLockImpl: deadlockLock, + }, + { + name: "stateLock-with-unrecognized", + input: "stateLock,otherLock", + expectedDetectDeadlockSlice: []string{"statelock", "otherlock"}, + expectedStateLockImpl: deadlockLock, + expectedAuthLockImpl: standardLock, + expectedMountsLockImpl: standardLock, + }, + } { + t.Run(tc.name, func(t *testing.T) { + core, _, _ := TestCoreUnsealedWithConfig(t, &CoreConfig{DetectDeadlocks: tc.input}) + + assert.ElementsMatch(t, tc.expectedDetectDeadlockSlice, core.detectDeadlocks) + + stateLockImpl := fmt.Sprintf("%T", core.stateLock) + authLockImpl := fmt.Sprintf("%T", core.authLock) + mountsLockImpl := fmt.Sprintf("%T", core.mountsLock) + + assert.Equal(t, tc.expectedStateLockImpl, stateLockImpl) + assert.Equal(t, tc.expectedAuthLockImpl, authLockImpl) + assert.Equal(t, tc.expectedMountsLockImpl, mountsLockImpl) + }) + } +} + func TestSetSeals(t *testing.T) { oldSeal := NewTestSeal(t, &seal.TestSealOpts{ StoredKeys: seal.StoredKeysSupportedGeneric, diff --git a/vault/logical_system.go b/vault/logical_system.go index 77b37dbf5cff..972a3b653ed5 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -2225,12 +2225,12 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, return nil, logical.ErrReadOnly } - var lock *locking.DeadlockRWMutex + var lock locking.RWMutex switch { case strings.HasPrefix(path, credentialRoutePrefix): - lock = &b.Core.authLock + lock = b.Core.authLock default: - lock = &b.Core.mountsLock + lock = b.Core.mountsLock } lock.Lock()