Re: [PATCH RFC v3 03/13] rcu-tasks: Add a Tasks RCU implementation for reader-marked trampolines

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rcu-tasks: Use-after-free in trampolines executed by the idle task
--

commit 4dc15eb660eecbf1c68e4f2d97dcdde7b8456c35
Author: Josef Bacik <josef@toxicpanda.com>

    rcu-tasks: Add a Tasks RCU implementation for reader-marked trampolines

    Tasks RCU waits for every task to pass through a voluntary context switch,
    usermode or idle, because a preempted task might be sitting in a trampoline
    that is about to be freed and nothing marks it as such. This adds a new
    Tasks RCU implementation for architectures that make their trampolines Tasks
    Trace RCU readers.

> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index e4acd50bd81a1..94318519998c8 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -141,8 +142,13 @@ void raw_irqentry_exit_cond_resched(struct pt_regs *regs)
>  		rcu_irq_exit_check_preempt();
>  		if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
>  			WARN_ON_ONCE(!on_thread_stack());
> -		if (need_resched() && arch_irqentry_exit_need_resched())
> +		if (need_resched() && arch_irqentry_exit_need_resched()) {
> +			if (IS_ENABLED(CONFIG_TASKS_RCU_TRAMPOLINE_READERS))
> +				rcu_tasks_irq_resched_enter(instruction_pointer(regs));

[Severity: High]
Does this logic leave the idle task off the holdout list if it is
interrupted while executing trampoline text?

If an IRQ interrupts the idle task (for example, while executing a traced
function before entering an RCU extended quiescent state) and enters the
trampoline's unmarked preamble, but does not wake any new tasks,
need_resched() will be false.

Because need_resched() is false, raw_irqentry_exit_cond_resched() will not
call rcu_tasks_irq_resched_enter(), meaning the idle task is never added to
the holdout list.

>  			preempt_schedule_irq();
> +			if (IS_ENABLED(CONFIG_TASKS_RCU_TRAMPOLINE_READERS))
> +				rcu_tasks_irq_resched_exit();
> +		}
>  	}
>  }

[ ... ]

> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 627295396cd91..3a7c092361a66 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h

[ ... ]

> +/* Has @cpu passed a quiescent event since the snapshot, or need it not? */
> +static bool rcu_tasks_cpu_quiescent(int cpu)
> +{
> +	if (!cpu_online(cpu))
> +		return true;
> +	/* Pairs with the release in rcu_tasks_qs_event(). */
> +	if (smp_load_acquire(per_cpu_ptr(&rcu_tasks_qs_seq, cpu)) !=
> +	    per_cpu(rcu_tasks_qs_snap, cpu))
> +		return true;
> +	/*
> +	 * Idle or nohz_full userspace (an RCU extended quiescent state): no
> +	 * task-level kernel frames there, and whatever ran before has switched
> +	 * out.  As with the classic flavor, the idle task itself is not waited
> +	 * for.
> +	 */
> +	if (!(ct_rcu_watching_cpu(cpu) & CT_RCU_WATCHING))
> +		return true;
> +	return idle_cpu(cpu);
> +}

[Severity: High]
Could the unconditional return of idle_cpu() lead to a use-after-free crash
if the idle task was interrupted in trampoline code?

Since the idle task wasn't added to the holdout list during IRQ exit
(because need_resched() was false in raw_irqentry_exit_cond_resched()), the
grace period thread relies entirely on rcu_tasks_cpu_quiescent().

Because idle_cpu(cpu) returns true, the QS sequence check for the idle task
is bypassed. The GP thread will assume the CPU has passed a quiescent state,
complete the grace period, and free the trampoline memory.

When the IRQ returns, the idle task will resume execution in the now-freed
trampoline memory, which can lead to a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-b4-rcu-tasks-preempt-qs-v3-0-0ad30c4c5ee7@toxicpanda.com?part=3