Re: [PATCH v6 03/14] 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:
- [Medium] rcu-tasks: Use-After-Free if task oopses inside irq-exit preemption
--

commit 6894dfa1f4c077dbeacd53dcfc82e1697f747465
Author: Josef Bacik <josef@toxicpanda.com>

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

This commit introduces a new Tasks RCU implementation for architectures
where trampolines are marked as Tasks Trace RCU readers. It tracks context
switches and preemptions during irq exits to wait for tasks that might
be lingering in unmarked trampoline text.

> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 627295396cd91..e7498601c28c3 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
[ ... ]
> +static void rcu_tasks_tramp_hold(struct task_struct *t)
> +{
> +	unsigned long flags;
> +
> +	if (t->rcu_tasks_holdout)
> +		return;
> +	raw_spin_lock_irqsave(&rcu_tasks_tramp_lock, flags);
> +	list_add_tail(&t->rcu_tasks_holdout_list, &rcu_tasks_tramp_holdouts);
> +	WRITE_ONCE(t->rcu_tasks_holdout, true);
> +	raw_spin_unlock_irqrestore(&rcu_tasks_tramp_lock, flags);
> +}

[Severity: Medium]
Does this code risk a use-after-free of the task struct?

It appears that the task is added to the holdout list without incrementing its
reference count (e.g., unlike the classic flavor which deliberately calls
get_task_struct()).

If a task is preempted inside unmarked trampoline text,
rcu_tasks_irq_resched_enter() adds it to rcu_tasks_tramp_holdouts. If the task
subsequently suffers a hardware exception or kernel oops while executing
preempt_schedule_irq(), it will be killed via make_task_dead() and do_exit().
Because rcu_tasks_irq_resched_exit() would never be reached to clear
t->rcu_tasks_irq_ip, rcu_tasks_note_qs() skips removing it from the holdout
list, leaving a dangling pointer when the task is later freed.

[ ... ]
> +static void rcu_tasks_tramp_wait_holdouts(struct rcu_tasks *rtp, unsigned long *lastreport)
> +{
> +	struct task_struct *t;
> +	unsigned long flags;
> +	int cpu;
> +
> +	raw_spin_lock_irqsave(&rcu_tasks_tramp_lock, flags);
> +	list_splice_tail_init(&rcu_tasks_tramp_holdouts, &rcu_tasks_gp_holdouts);
> +	raw_spin_unlock_irqrestore(&rcu_tasks_tramp_lock, flags);
> +
> +	for (;;) {
> +		struct cpumask *kick = &rcu_tasks_pending_cpus;
> +		struct task_struct *show[8];
> +		int nshow = 0, i;
> +		bool empty, report;
> +
> +		report = rcu_tasks_tramp_stall(rtp, lastreport,
> +					       "tasks preempted in trampoline text");
> +		cpumask_clear(kick);
> +		raw_spin_lock_irqsave(&rcu_tasks_tramp_lock, flags);
> +		empty = list_empty(&rcu_tasks_gp_holdouts);
> +		list_for_each_entry(t, &rcu_tasks_gp_holdouts, rcu_tasks_holdout_list) {
> +			if (task_curr(t))
> +				__cpumask_set_cpu(task_cpu(t), kick);

[Severity: Medium]
If the scenario described above occurs and the task struct has been freed,
could we hit a use-after-free here when iterating over the holdout list and
accessing t?

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