kanodayo1111 opened a new issue, #19808: URL: https://github.com/apache/nuttx/issues/19808
### Description / Steps to reproduce the issue ## Summary With `CONFIG_TICKET_SPINLOCK=y`, a failed `spin_trylock_notrace()` can overwrite the ticket lock's `owner` counter. This can make a lock held by another CPU appear unlocked and allow concurrent entry into its critical section. ## Affected revision - Repository: https://github.com/open-vela/nuttx - Commit: `dd92bcf425738734d1b8aed09c2bd4dbe3f2e438` - Commit date: 2026-07-10 - Function: `spin_trylock_notrace()` - File: `include/nuttx/spinlock.h` - Affected lines: 259-260 Affected code: ```c #ifdef CONFIG_TICKET_SPINLOCK return atomic_cmpxchg_acquire(&lock->next, &lock->owner, atomic_read(&lock->next) + 1); #else /* CONFIG_TICKET_SPINLOCK */ ``` Permalink: https://github.com/open-vela/nuttx/blob/dd92bcf425738734d1b8aed09c2bd4dbe3f2e438/include/nuttx/spinlock.h#L259-L260 ## Configuration The repository includes a configuration that enables ticket spinlocks: - Board/configuration: `sim:sim:smp` - Defconfig: `boards/sim/sim/sim/configs/smp/defconfig` - Relevant options: ```text CONFIG_SMP=y CONFIG_TICKET_SPINLOCK=y ``` `CONFIG_SPINLOCK` is selected as part of the SMP configuration. ## Root cause `atomic_cmpxchg_acquire()` implements compare-exchange semantics. Its second argument is an input/output pointer to the expected value: - Before the operation, `*expected` specifies the value expected in the target object. - If the compare-exchange fails, the implementation writes the target's actual value back through `expected`. The atomic wrapper documents this contract through `atomic_compare_exchange_4()`: ```c #define atomic_cmpxchg_acquire(obj, expected, desired) \ atomic_compare_exchange_4(obj, (FAR int32_t *)expected, desired, false, \ __ATOMIC_ACQUIRE, __ATOMIC_RELAXED) ``` In the ticket-lock implementation, `&lock->owner` is passed as the `expected` storage: ```c atomic_cmpxchg_acquire(&lock->next, &lock->owner, ...); ``` Therefore, when the CAS fails, it updates `lock->owner` with the current value of `lock->next`. `owner` is part of the lock state and must only be advanced by the lock holder during unlock. ## Failure scenario A ticket lock is held when `owner != next`. Initial state: ```text owner = 0 next = 1 ``` CPU 0 owns ticket `0` and is still executing its critical section. CPU 1 calls `spin_trylock_notrace()`. The current code effectively performs: ```c atomic_cmpxchg_acquire(&lock->next, &lock->owner, 2); ``` The operation fails because: ```text next = 1 owner = 0 ``` On failure, compare-exchange stores the actual value of `next` into the supplied expected-value location, which is `owner`: ```text owner = 1 next = 1 ``` The ticket lock now appears unlocked because `owner == next`, despite CPU 0 still owning the critical section. A subsequent CPU can acquire the lock and enter the critical section concurrently with CPU 0. ## Impact This breaks the mutual-exclusion guarantee of ticket spinlocks. The issue is reachable when all of the following are true: - `CONFIG_TICKET_SPINLOCK=y` - A ticket spinlock is currently held - Another CPU or execution context calls `spin_trylock()` or a wrapper using `spin_trylock_notrace()` Potential consequences depend on the protected data, including memory corruption, inconsistent scheduler or driver state, and hardware register access races. ## Suggested fix Use a local variable for the compare-exchange expected value. The local value must represent the observed `owner` value and must not alias a member of the lock state. ```c #ifdef CONFIG_TICKET_SPINLOCK atomic_t expected = atomic_read(&lock->owner); return atomic_cmpxchg_acquire(&lock->next, &expected, expected + 1); #else /* CONFIG_TICKET_SPINLOCK */ return atomic_xchg_acquire(&lock->lock, 1) != 1; #endif /* CONFIG_TICKET_SPINLOCK */ ``` This succeeds only when `next == owner`, which is the unlocked state of a ticket lock. If the CAS fails, the implementation updates only the local `expected` variable and leaves `lock->owner` unchanged. The CAS remains necessary because another CPU may acquire the lock after the caller reads `owner` and before it updates `next`. ### On which OS does this issue occur? [OS: Linux] ### What is the version of your OS? Ubuntu 22.04.5 LTS ### NuttX Version OpenVela NuttX master (commit 322ad9f11c333315356e3e4c8f3b64e5121cfc6d) ### Issue Architecture [Arch: arm] ### Issue Area [Area: Kernel] ### Host information _No response_ ### Verification - [x] I have verified before submitting the report. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
