Re: [PATCH v1] irq_work: Fix use-after-free in irq_work_single on PREEMPT_RT

[Sebastian Andrzej Siewior]

On 2026-03-26 00:34:58 [+0800], Jiayuan Chen wrote:
> In irq_work_single(), just wrap the post-callback section with
> rcu_read_lock to keep the work structure alive through an RCU grace
> period:
> 
> '''
>   lockdep_irq_work_enter(flags);
>   work->func(work);
>   lockdep_irq_work_exit(flags);
> 
> +   rcu_read_lock();
> 
>   (void)atomic_cmpxchg(&work->node.a_flags, flags, flags & ~IRQ_WORK_BUSY);
> 
>   if ((IS_ENABLED(CONFIG_PREEMPT_RT) && !irq_work_is_hard(work)) ||
>       !arch_irq_work_has_interrupt())
>           rcuwait_wake_up(&work->irqwait);
> 
> +   rcu_read_unlock();
> '''

This shouldn't be needed. run_irq_workd() should get a guard(rcu)(),
the other callers run with disabled interrupts. This should include CPU
hotplug invocation but need to check.

> Then provide a helper for callers that need to free:
> 
> void irq_work_synchronize_free(struct irq_work *work)
> {
>   irq_work_sync(work);
>   synchronize_rcu();
> }

Why not just having the synchronize_rcu()?

> Callers that free the containing structure would switch to
> irq_work_synchronize_free(), or use kfree_rcu() if appropriate

If we provide the irq_work_synchronize_free() then using kfree_rcu()
would sort of open code irq_work_synchronize_free() since we couldn't
simply replace synchronize_rcu() with something else and update the
irq_work core side (we would also have to update all users). I guess
that was Steven's idea in providing a helper for synchronisation.

> Thanks,
> Jiayuan

Sebastian