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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bypass of debug validation causes fatal RCU list corruption on double-free.
- [High] O(N) runqueue spinlock acquisitions cause severe interrupt latency during callback drain.
--

--- Patch [2]: [PATCH v3 2/6] rcu: Make Tiny call_rcu() safe to call from any context ---
commit 695767be85ec82f2457b846af53d8afe8fb742d1
Author: Puranjay Mohan <puranjay@kernel.org>

rcu: Make Tiny call_rcu() safe to call from any context

This commit updates Tiny RCU's call_rcu() to defer callbacks when called
with interrupts disabled. It uses a lockless list and an irq_work to
re-issue them later, similar to Tree RCU.

> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index dccccd6be9411..baffe660043f5 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c

[ ... ]

> +static void rcu_do_enqueue(struct rcu_head *head, rcu_callback_t func)
> +{
> +	static atomic_t doublefrees;
> +	unsigned long flags;
> +
> +	if (debug_rcu_head_queue(head)) {
> +		if (atomic_inc_return(&doublefrees) < 4) {
> +			pr_err("%s(): Double-freed CB %p->%pS()!!!  ", __func__, head, head->func);
> +			mem_dump_obj(head);
> +		}
> +		return;
> +	}
> +
> +	head->func = func;
> +	head->next = NULL;
> +
> +	local_irq_save(flags);
> +	*rcu_ctrlblk.curtail = head;
> +	rcu_ctrlblk.curtail = &head->next;
> +	local_irq_restore(flags);
> +
> +	if (unlikely(is_idle_task(current))) {
> +		/* force scheduling for rcu_qs() */
> +		resched_cpu(0);
> +	}
> +}
> +
> +static void __rcu_defer_drain(bool guard)
> +{
> +	struct llist_node *node, *next;
> +	unsigned long flags;
> +
> +	/* Callbacks are unordered, so drain in llist order without reversing. */
> +	local_irq_save(flags);
> +	if (guard)
> +		WRITE_ONCE(rcu_defer_draining, true);
> +	llist_for_each_safe(node, next, llist_del_all(&rcu_defer_list)) {
> +		struct rcu_head *head = (struct rcu_head *)node;
> +
> +		head->next = NULL;
> +		rcu_do_enqueue(head, head->func);

[Severity: High]
Does this code cause severe interrupt latency during callback drain?

If a batch of RCU callbacks is deferred and the CPU subsequently goes idle,
__rcu_defer_drain() loops over the entire batch in hardirq context.

For each callback, rcu_do_enqueue() will call resched_cpu(0) because
current is the idle task.  Since resched_cpu() unconditionally takes the
runqueue spinlock, this results in an O(N) locking sequence in hardirq
context, which could lead to watchdog soft lockups.

> +	}
> +	if (guard)
> +		WRITE_ONCE(rcu_defer_draining, false);
> +	local_irq_restore(flags);
> +}

[ ... ]

> @@ -157,29 +250,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 (debug_rcu_head_queue(head)) {
> -		if (atomic_inc_return(&doublefrees) < 4) {
> -			pr_err("%s(): Double-freed CB %p->%pS()!!!  ", __func__, head, head->func);
> -			mem_dump_obj(head);
> -		}
> +	if (should_rcu_defer()) {
> +		call_rcu_defer(head, func);

[Severity: High]
Does moving the debug validation after the deferral bypass it during a
double-free?

If a subsystem double-frees an RCU callback while interrupts are disabled,
it will take this deferral path instead of checking debug_rcu_head_queue().

In call_rcu_defer():

	if (llist_add((struct llist_node *)head, &rcu_defer_list))

If the callback is already in the list, llist_add() modifies its next
pointer, creating a cycle in the lockless list.  Later, when the
irq_work drains the list, llist_for_each_safe() in __rcu_defer_drain()
will loop infinitely, causing a system hard lockup in hardirq context.

>  		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());
>  
> -	if (unlikely(is_idle_task(current))) {
> -		/* force scheduling for rcu_qs() */
> -		resched_cpu(0);
> -	}
> +	rcu_do_enqueue(head, func);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805122346.269445-1-puranjay@kernel.org?part=2