Re: [RFC][PATCH] make sure that lock_for_kill() callers drop the locks in safe order

Paul E. McKenney
2026-04-10T18:11:05Z

On Fri, Apr 10, 2026 at 10:31:32AM -0700, Linus Torvalds wrote:
> On Fri, 10 Apr 2026 at 09:27, Boqun Feng <boqun@kernel.org> wrote:
> >
> > Notice in rcu_read_unlock_special(), atomicity/preemption count is
> > checked in two ways:
>
> Yeah, I didn't go that deep ... That code is inscrutable.

We are working to make it better, but this will be a long process.

> Raising a softirq looks pretty strangein there, since there's
> absolutely no reason to believe that the preemption-off region has
> ended before the softirq is triggered. It makes sense for the "we're
> in hardirq context", but it also does it for that (needs_exp &&
> !irqs_were_disabled()) case
>
> So then it depends on the softirq checking it *again*. That all seems
> a bit strange.
>
> But it looks like rcu_core_si() does do that, and then falls back to
>
> if (rcu_preempt_need_deferred_qs())
> set_need_resched_current();
>
> and it feels like maybe the rcu_read_unlock_special() should just have
> done that itself instead of playing softirq games.
>
> Maybe it's some off optimization, but going through the softirq code
> doesn't _feel_ like it's the right thing to do.
>
> I don't know this code well enough to judge.

We could quite easily add another leg to the inner "if" statement
in rcu_read_unlock_special() so that it does raise_softirq_irqoff()
if in_hardirq() and set_need_resched_current() otherwise, as in the
(untested, probably doesn't even build)first patch at the end of this
email. My feeling is that it is not worth it, but on the other hand,
my idea of "simplicity" might or might not serve the people reading
this code.

> Let's wait to see if Al's patch actually makes any difference. It
> *shouldn't* make any difference, and maybe it doesn't. No need to
> start chasing RCU bugs that may not even be there.

Fair enough!

> Jeff?
>
> > @Jeff, just want to confirm, this UAF can be reproduced without
> > PREEMPT_RT=y, right?
>
> I haven't seen Jeff's config, but some of the syzbot reports that
> *may* be related have configs that look pretty normal and didn't have
> PREEMPT_RT.

And I am concerned about PREEMPT_RT, please see the second patch below.
But Sebastian will no doubt tell me the error of my ways. My question is
"why is it safe to exit the RCU reader before the lock is fully released".
Sebastian's answer might well be that all the valid use cases for
combining locking and RCU have a lock acquisition on the free path.
Whether I believe that or not, I have no idea. ;-)

------------------------------------------------------------------------

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 95ad967adcf3c..066b6fe6b6a3f 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -749,7 +749,10 @@ static void rcu_read_unlock_special(struct task_struct *t)
// GP in flight or a potential need to deboost.
if (rdp->defer_qs_pending != DEFER_QS_PENDING) {
rdp->defer_qs_pending = DEFER_QS_PENDING;
- raise_softirq_irqoff(RCU_SOFTIRQ);
+ if (in_hardirq())
+ raise_softirq_irqoff(RCU_SOFTIRQ);
+ else
+ set_need_resched_current();
}
} else {
// Enabling BH or preempt does reschedule, so...

------------------------------------------------------------------------

diff --git a/kernel/locking/spinlock_rt.c b/kernel/locking/spinlock_rt.c
index db1e11b45de67..fbf4deba4029c 100644
--- a/kernel/locking/spinlock_rt.c
+++ b/kernel/locking/spinlock_rt.c
@@ -79,10 +79,10 @@ 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);
+ rcu_read_unlock();
}
EXPORT_SYMBOL(rt_spin_unlock);

@@ -262,17 +262,17 @@ void __sched rt_read_unlock(rwlock_t *rwlock) __releases(RCU)
{
rwlock_release(&rwlock->dep_map, _RET_IP_);
migrate_enable();
- rcu_read_unlock();
rwbase_read_unlock(&rwlock->rwbase, TASK_RTLOCK_WAIT);
+ rcu_read_unlock();
}
EXPORT_SYMBOL(rt_read_unlock);

void __sched rt_write_unlock(rwlock_t *rwlock) __releases(RCU)
{
rwlock_release(&rwlock->dep_map, _RET_IP_);
- rcu_read_unlock();
migrate_enable();
rwbase_write_unlock(&rwlock->rwbase);
+ rcu_read_unlock();
}
EXPORT_SYMBOL(rt_write_unlock);