Re: rt_spin_unlock order of operations [was: Re: [syzbot] [fs?] KASAN: slab-use-after-free Read in shrink_dcache_tree]

[Al Viro]

On Thu, Jun 18, 2026 at 08:44:32PM +0200, Jann Horn wrote:
> I think this is more of a bug in RT spinlocks than a VFS bug, though
> it's a bit murky.
> 
> rt_spin_unlock() looks like this:
> 
> void __sched rt_spin_unlock(spinlock_t *lock) __releases(RCU)
> {
>         spin_release(&lock->dep_map, _RET_IP_);
>         migrate_enable();
>         rcu_read_unlock();
> 
>         if (unlikely(!rt_mutex_cmpxchg_release(&lock->lock, current, NULL)))
>                 rt_mutex_slowunlock(&lock->lock);
> }
> 
> Note how the RCU read-side critical section and the protection against
> migration end *before* the lock is actually released, which means this
> can UAF if the RCU read-side critical section implied by the spinlock
> is the only thing keeping the lock alive. While non-RT spinlocks do
> this the other way around (do_raw_spin_unlock() before
> preempt_enable()):
> 
> static inline void __raw_spin_unlock(raw_spinlock_t *lock)
>         __releases(lock)
> {
>         spin_release(&lock->dep_map, _RET_IP_);
>         do_raw_spin_unlock(lock);
>         preempt_enable();
> }
> 
> https://docs.kernel.org/next/RCU/whatisRCU.html guarantees that
> spinlock APIs imply RCU, and
> https://docs.kernel.org/locking/mutex-design.html says: "This is in
> contrast with spin_unlock() [...], which APIs can be used to guarantee
> that the memory is not touched by the lock implementation after
> spin_unlock()/completion_done() releases the lock.".
> Neither of these explicitly guarantees that the RCU read-side critical
> section (and the protection against migration?) should still hold
> while the lock is being dropped, but I think that would fit best with
> the explicit guarantees?

I'm trying to recall if PREEMPT_RT had been enabled in the last round of
UAF in that area back in early April...

As far as I'm concerned, we *do* need to keep RCU read-side critical area
all the way until the end of spin_unlock(); it very well might be the
only thing to prevent freeing the sucker under us.