Re: [PATCH v2 2/6] rcu: Make Tiny call_rcu() safe to call from any context

[Puranjay Mohan]

On Mon, Aug 3, 2026 at 3:35 PM <sashiko-bot@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Relocating `debug_rcu_head_queue()` into `rcu_do_enqueue()` bypasses double-free detection for deferred callbacks, silently corrupting the lockless list and inducing a permanent hard livelock during `rcu_defer_drain()`.
> - [Medium] Invalid `WARN_ON_ONCE` in `call_rcu()` falsely flags safe NMI executions due to a flawed concurrency assumption.
> --
>
> --- Patch [2]: [PATCH v2 2/6] rcu: Make Tiny call_rcu() safe to call from any context ---
> commit 866e1596ae5a7314657b9b0164dfabf922016c70
> Author: Puranjay Mohan <puranjay@xxxxxxxxxx>
>
> rcu: Make Tiny call_rcu() safe to call from any context
>
> Give Tiny call_rcu() the same treatment as Tree RCU.  When interrupts are
> disabled and the scheduler is up, stage the callback on a lockless list
> that an irq_work re-issues later.  One global list and irq_work suffice
> since Tiny RCU is uniprocessor, and there is no CPU-offline drain.
>
> > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > index dccccd6be9411..5736b964d8ee1 100644
> > --- a/kernel/rcu/tiny.c
> > +++ b/kernel/rcu/tiny.c
>
> [ ... ]
>
> > @@ -42,8 +44,86 @@ static struct rcu_ctrlblk rcu_ctrlblk = {
>
> [ ... ]
>
> > +static void call_rcu_defer(struct rcu_head *head, rcu_callback_t func)
> > +{
> > +     /* A re-entrant call_rcu() during the drain would livelock it; drop it. */
> > +     if (rcu_defer_draining && !in_nmi()) {
> > +             WARN_ONCE(1, "call_rcu() re-entered during callback drain; leaking callback\n");
> > +             return;
> > +     }
> > +     head->func = func;
> > +     if (llist_add((struct llist_node *)head, &rcu_defer_list))
>
> [Severity: High]
> Does moving the debug_rcu_head_queue() check into rcu_do_enqueue() bypass
> double-free detection for deferred callbacks?
>
> If a double-free occurs, llist_add() will be called twice on the same node here,
> corrupting the lockless list by creating a circular link (head->next = head).
>
> When the irq_work executes rcu_defer_drain(), won't the llist_for_each_safe()
> iteration infinitely hang on the circular list before the delayed debug check
> can prevent the livelock?
>
> > +             irq_work_queue(&rcu_defer_iw);
> > +}
>
> [ ... ]
>
> > @@ -157,29 +237,15 @@ EXPORT_SYMBOL_GPL(synchronize_rcu);
> >   */
> >  void call_rcu(struct rcu_head *head, rcu_callback_t func)
> >  {
> > -     static atomic_t doublefrees;
> > -     unsigned long flags;
>
> [ ... ]
>
> > +     if (should_rcu_defer()) {
> > +             call_rcu_defer(head, func);
> >               return;
> >       }
> >
> > -     head->func = func;
> > -     head->next = NULL;
> > -
> > -     local_irq_save(flags);
> > -     *rcu_ctrlblk.curtail = head;
> > -     rcu_ctrlblk.curtail = &head->next;
> > -     local_irq_restore(flags);
> > +     /* An NMI reaching here entered with irqs enabled, so the enqueue can race. */
> > +     WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && in_nmi());
>
> [Severity: Medium]
> Is this concurrency assumption backwards?
>
> If an NMI interrupted a context where IRQs were enabled, that interrupted
> context could not have been in the middle of rcu_do_enqueue() because
> rcu_do_enqueue() disables IRQs via local_irq_save().
>
> Since it's impossible for the NMI to race with rcu_do_enqueue() in this
> scenario, will this falsely trigger kernel warnings during safe NMI
> executions?

This code assumes that irqs_disabled() returns true in NMI handlers,
so the execution should not reach this warning in NMI handlers.