Re: [PATCH v4 1/6] rcu: Make call_rcu() safe to call from any context

On Mon, Aug 10, 2026 at 1:43 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Deferred callbacks bypass `debug_rcu_head_queue()` check, causing immediate list corruption on double `call_rcu()` and defeating `CONFIG_DEBUG_OBJECTS_RCU_HEAD`.
> --
>
> --- Patch [1]: [PATCH v4 1/6] rcu: Make call_rcu() safe to call from any context ---
> commit 5205f0afe602a3f93ee865a26d94981f93553b7d
> Author: Puranjay Mohan <[email protected]>
>
> rcu: Make call_rcu() safe to call from any context
>
> This patch defers call_rcu() via a per-CPU llist and irq_work when called
> with interrupts disabled. This avoids deadlocks or list corruption if
> call_rcu() is invoked from NMI or instrumentation.
>
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 96848fc1f02b8..ff9a2395c9e8e 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
>
> [ ... ]
>
> > -static void
> > -__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> > +/*
> > + * Also called by __rcu_defer_drain() to re-issue a deferred callback, so it
> > + * must not re-check the deferral condition.  Either caller may have interrupts
> > + * already disabled, and a drain of a remote CPU re-issues onto the draining
> > + * CPU.
> > + */
> > +static void rcu_do_enqueue(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> >  {
> >       static atomic_t doublefrees;
> >       unsigned long flags;
> >       bool lazy;
> >       struct rcu_data *rdp;
> >
> > -     /* Misaligned rcu_head! */
> > -     WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));
> > -
> > -     /* Avoid NULL dereference if callback is NULL. */
> > -     if (WARN_ON_ONCE(!func))
> > -             return;
> > -
> >       if (debug_rcu_head_queue(head)) {
>
> [ ... ]
>
> > +static void call_rcu_defer(struct rcu_head *head, rcu_callback_t func)
> > +{
> > +     struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> > +
> > +     /*
> > +      * Instrumentation on the enqueue path can re-enter here from inside the
> > +      * drain.  Re-queuing would livelock it, so drop the callback; an NMI
> > +      * cannot loop, so let it through.
> > +      */
> > +     if (READ_ONCE(rdp->defer_draining) && !in_nmi()) {
> > +             WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU),
> > +                       "call_rcu() re-entered during callback drain; leaking callback\n");
> > +             return;
> > +     }
> > +     head->func = func;
> > +     if (llist_add((struct llist_node *)head, &rdp->defer_head))
> > +             irq_work_queue(&rdp->defer_work);
> > +}
>
> [ ... ]
>
> > +static void
> > +__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> > +{
> > +     /* Misaligned rcu_head! */
> > +     WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));
> > +
> > +     /* Avoid NULL dereference if callback is NULL. */
> > +     if (WARN_ON_ONCE(!func))
> > +             return;
> > +
> > +     /* Record the caller: the irq_work's stack says nothing about it. */
> > +     kasan_record_aux_stack(head);
> > +
> > +     if (should_rcu_defer()) {
> > +             call_rcu_defer(head, func);
>
> [Severity: High]
> Does this bypass the debug check for double call_rcu(), potentially corrupting
> the callback list?
>
> By moving debug_rcu_head_queue() from __call_rcu_common() into rcu_do_enqueue(),
> deferred callbacks are added to the defer_head list before being validated.
>
> If a double call_rcu() occurs, llist_add() in call_rcu_defer() will overwrite
> head->next. If the callback was already in a normal cblist, its next pointer is
> overwritten with defer_head->first, immediately splitting the cblist and
> splicing it into the deferred list.
>
> Could we check debug_rcu_head_queue() here before deferring, so that
> CONFIG_DEBUG_OBJECTS_RCU_HEAD can safely catch the bug without list corruption?

No we can't debug_rcu_head_queue() can't be done from NMI and doubling
calling call_rcu() from NMI or interrupts disabled will result in
leaking some callbacks but it will not cause a deadlock (validated in
testing using a kernel module that does multiple call_rcu() from NMI).