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

[sashiko-bot]

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?

> -	if (unlikely(is_idle_task(current))) {
> -		/* force scheduling for rcu_qs() */
> -		resched_cpu(0);
> -	}
> +	rcu_do_enqueue(head, func);
>  }
>  EXPORT_SYMBOL_GPL(call_rcu);

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