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 <[email protected]> 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-0ad30c4c5...@toxicpanda.com?part=3
