Re: [PATCH 2/3] irq_work: Flush lazy work CPU down on PREEMPT_RT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Executing a dying CPU's lazy irq_work callbacks on the control CPU violates `this_cpu_*()` assumptions, leading to memory leaks and state corruption.
- [High] Use-after-free of `irq_work` structure due to missing `rcu_read_lock()` in `irq_work_run_cpu()`.
--

commit c2c4af12bef2f74291db156f37cadb81d93b8598
Author: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

irq_work: Flush lazy work CPU down on PREEMPT_RT

This commit adds support for flushing lazy IRQ work for a dead CPU when
PREEMPT_RT is enabled. It introduces irq_work_run_cpu() to process the
dying CPU's lazy list from the control CPU during smpcfd_dead_cpu().

> While looking through the users of irq_work, it does not matter if the
> callback is enqueued on another CPU. This makes it possible to invoke
> the callbacks from a remote CPU by smpcfd_dead_cpu().

[Severity: High]
Is this statement accurate?

Several lazy irq_work callbacks (e.g., cgrp_dead_tasks_iwork_fn in
kernel/cgroup/cgroup.c) rely on this_cpu_*() accessors to process local queues:

kernel/cgroup/cgroup.c:cgrp_dead_tasks_iwork_fn() {
    ...
    lnode = llist_del_all(this_cpu_ptr(&cgrp_dead_tasks));
    ...
}

Executing them remotely on the control CPU causes them to access and drain the
control CPU's per-CPU lists instead of the dying CPU's. Could this permanently
leak task structs and pin cgroups on the dead CPU indefinitely, while
incorrectly altering the state of the control CPU?

> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index 9f19c05c9962c..73eabcbdcd50c 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
[ ... ]
> @@ -263,6 +263,17 @@ void irq_work_run(void)
>  }
>  EXPORT_SYMBOL_GPL(irq_work_run);
>  
> +void irq_work_run_cpu(unsigned int cpu)
> +{
> +	if (WARN_ON_ONCE(!cpumask_test_cpu(cpu, cpu_dying_mask)))
> +		return;
> +
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +		return;
> +
> +	irq_work_run_list(per_cpu_ptr(&lazy_list, cpu));
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Could this result in a use-after-free of the irq_work structure?

irq_work_sync() protects against concurrent execution by waiting for
IRQ_WORK_BUSY to clear and then calling synchronize_rcu(). This assumes the work
callback executes inside an RCU read-side critical section.

However, irq_work_run_cpu() runs from preemptible thread context in
smpcfd_dead_cpu() without holding the RCU read lock. irq_work_single() can be
preempted after clearing IRQ_WORK_BUSY but before calling rcuwait_wake_up().

If irq_work_sync() on another CPU sees the BUSY bit clear, it will call
synchronize_rcu() which returns immediately, and the caller may then free the
irq_work structure. When the preempted work function resumes, could it access
freed memory in rcuwait_wake_up()?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911143815.997254-1-bigeasy@xxxxxxxxxxxxx?part=2