Skip to content

[BUG] RP2040: task-lock fast path missing acquire barrier #1430

@TING0419

Description

@TING0419

Describe the bug

vPortRecursiveLock() in portable/ThirdParty/GCC/RP2040/include/portmacro.h (current main)
acquires the SIO task lock on the uncontended fast path via spin_try_lock_unsafe() — a bare
SIO register read (return *lock;) with no memory ordering. The acquire barrier
(__mem_fence_acquire() == __dmb()) lives only inside spin_lock_unsafe_blocking(), i.e. on the
contended path. When spin_try_lock_unsafe() succeeds (the common case), control falls straight
through to the lock-protected bookkeeping with no acquire barrier, so the new holder has no
happens-before edge to the previous holder's critical-section writes and may observe stale state
under weak memory.

portGET_TASK_LOCK drives taskENTER_CRITICAL(), so this affects every kernel structure read after
the lock is taken — it surfaces first on the lock's own ucRecursionCountByLock. This is a C11 data
race (undefined behavior); the access has no happens-before regardless of whether a given core
actually reorders.

Code (current main):

if (uxAcquire) {
    if (!spin_try_lock_unsafe(pxSpinLock)) {          // fast path: bare register read, NO ordering
        if (ucOwnedByCore[xCoreID][ulLockNum]) { ... return; }   // recursive
        spin_lock_unsafe_blocking(pxSpinLock);        // contended: spins, THEN __mem_fence_acquire()
    }
    /* FAST PATH falls through here with NO acquire barrier */
    configASSERT(ucRecursionCountByLock[ulLockNum] == 0);   // reads protected metadata
    ucRecursionCountByLock[ulLockNum] = 1;
    ucOwnedByCore[xCoreID][ulLockNum] = 1;
}

This is a portability/correctness defect against the weak-memory contract the SIO spinlock targets —
present even on a core whose in-order pipeline happens to mask the reorder (see the To Reproduce note).

Target

  • Development board: Raspberry Pi Pico (RP2040, dual Cortex-M0+)
  • Instruction Set Architecture: Armv6-M
  • IDE and version: N/A — analysis against upstream source, not a board build
  • Toolchain and version: N/A — defect identified by source review + formal memory-model checking

Host

  • Host OS: Linux
  • Version: N/A

To Reproduce

Note: this was found and verified at the memory-model level, not by observing a failure on
physical RP2040 hardware. RP2040 is in-order (Cortex-M0+), so the reorder may be masked in practice;
the C11 data race (UB) is present regardless.

Tools: GenMC v0.17.0 (RC11) and herd7 (AArch64 model).

bash harness/run_rq1.sh         # GenMC: faithful=race, broken=race, fix=clean  (3/3)
herd7 litmus/RQ1_handoff_noacq.litmus   # Observation Sometimes 1 3  (stale read allowed)
herd7 litmus/RQ1_handoff_acq.litmus     # Observation Never     0 3  (forbidden with the fix)

Minimal handoff litmus — writer does STR data; DMB; STR lock=0, reader does LDR lock; LDR data
with no acquire between, checking exists (reader sees lock==0 /\ data==stale). GenMC
counterexample: core0's acquiring CAS reads-from core1's release (mutual exclusion holds), but with
no trailing acquire fence it does not synchronize-with the release, so core1's critical-section write
is not happens-before core0's read → race.

Expected behavior

The lock holder acquired via the fast path should have an acquire barrier before reading
lock-protected state, establishing happens-before with the previous holder's release — the same
ordering the contended path already gets from __mem_fence_acquire().

Screenshots

N/A.

Additional context

Fix is one line, self-contained in the FreeRTOS port — no pico-sdk change required
(__mem_fence_acquire() is already used in this same file inside spin_lock_unsafe_blocking()):

        spin_lock_unsafe_blocking(pxSpinLock);
    }
    __mem_fence_acquire();   /* fast path carries no acquire ordering */
    configASSERT(ucRecursionCountByLock[ulLockNum] == 0);

Unconditional, so the contended path gets one redundant (harmless) __dmb(). A no-redundancy
variant puts the fence in an else of the try-lock if (fast path only). Equivalent root-cause
alternatives, if preferred: give spin_try_lock_unsafe() acquire semantics in pico-sdk, or add a
fencing try-lock variant.

I've verified both the defect and the fix locally with GenMC (RC11) and herd7 (AArch64). Happy to
submit a PR for the portmacro.h fast-path fence — or defer to a pico-sdk fix if you prefer that
layer. Full write-up + counterexample trace available on request.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions