Re: [PATCH 6.18.y 5/6] rust: sync: Add SpinLockIrq

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] workqueue: missing flush_work in spinlock_irq test causes module-unload UAF
- [Medium] rust: sync: lockdep_assert_irqs_disabled() in test fails on PREEMPT_RT
- [Low] rust: sync: missing loop around CondVar::wait in spinlock_irq_condvar test
- [Low] rust: sync: misleading SpinLockIrqBackend documentation
--

commit 53307f4fda1c118612a5cc4ab386ba1ddc8eda26
Author: Lyude Paul <lyude@redhat.com>

rust: sync: Add SpinLockIrq

A variant of `SpinLock` that ensures interrupts are disabled in the
critical section. `lock()` will ensure that either interrupts are
already disabled or disable them. `unlock()` will reverse the respective
operation.

Link: https://patch.msgid.link/20260807070218.27144-18-boqun@kernel.org

> diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
> --- a/rust/kernel/sync/lock/spinlock.rs
> +++ b/rust/kernel/sync/lock/spinlock.rs
[ ... ]
> +/// A kernel `spinlock_t` lock backend that can only be acquired in interrupt disabled contexts.
> +pub struct SpinLockIrqBackend;

[Severity: Low]
Does this comment accurately reflect the backend's behavior?

The backend implementation calls bindings::spin_lock_irq_disable(ptr),
which disables interrupts itself if they aren't already disabled.

Could this doc string be updated to clarify that it disables interrupts,
rather than requiring them to be disabled beforehand?

[ ... ]
> +    impl WorkItem for Test {
> +        type Pointer = Arc<Self>;
> +
> +        fn run(this: Arc<Self>) {
> +            // Wait for the test to be ready to wait for us
> +            let mut state = this.state.lock();
> +
> +            // Make sure the interrupts actually turned off
> +            // SAFETY: It's always safe to call `lockdep_assert_irqs_disabled()`
> +            unsafe { bindings::lockdep_assert_irqs_disabled() };

[Severity: Medium]
Will this test fail on preempt-rt configurations?

On preempt-rt, spin_lock_irq_disable() is implemented as a sleepable lock
and intentionally does not disable hardware interrupts. This means
bindings::lockdep_assert_irqs_disabled() will evaluate to true and trigger
a warning or panic depending on configuration.

Could this check be adjusted or guarded so the test doesn't unconditionally
fail on preempt-rt kernels?

[ ... ]
> +    #[test]
> +    fn spinlock_irq_condvar() -> Result {
> +        let testdata = Test::new()?;
> +
> +        let _ = workqueue::system().enqueue(testdata.clone());
> +
> +        // Let the updater know when we're ready to wait
> +        let mut state = testdata.state.lock();
> +        state.waiter_ready = true;
> +        testdata.waiter_state_changed.notify_sync();
> +
> +        // Wait for the exciting value update
> +        testdata.state_changed.wait(&mut state);

[Severity: Low]
Is this wait vulnerable to spurious wakeups?

Condition variables in the kernel waitqueue can wake up spuriously.
If a spurious wakeup occurs before the worker thread increments the value,
the test will proceed to the assert and panic since the value is still 1.

Should this wait be wrapped in a while loop checking state.value,
similar to how the worker thread checks state.waiter_ready?

> +        assert_eq!(state.value, 2);
> +        Ok(())
> +    }

[Severity: High]
Does this test leave background work running, potentially causing a
use-after-free during module unload?

The test enqueues a work item to workqueue::system() and waits for
state_changed.notify_sync(). Once notified, this main thread completes and
returns, which allows the kunit test to finish.

If the module is unloaded immediately after the test completes, the worker
thread executing Test::run() might still be finishing its final steps or
returning to the C workqueue executor. This would cause the kernel to
execute unmapped memory.

Should there be a flush_work() or similar synchronization barrier here
before returning from the test?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-spinlock-irq-6-18-v1-0-72c58975994e@google.com?part=5