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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 3/25/26 11:55 PM, Sebastian Andrzej Siewior wrote:
> On 2026-03-25 11:53:15 [-0400], Steven Rostedt wrote:
>> On Wed, 25 Mar 2026 16:38:26 +0100
>> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>>
>>> Most irq-work aren't free()ed since they are static and remain around.
>>> There is no task assigned if there is no active waiter.
>>> Wouldn't it be easier to kfree_rcu() the struct using the irq-work?
>> I guess we should add some kind of helper then. Like tracepoints have.
>>
>>     tracepoint_synchronize_unregister()
>>
>> Perhaps have a:
>>
>>     irq_work_synchronize_free();
>>
>> Or something like that to let developers know that they just can't safely free a
>> structure that contains an irq_work?
> That sounds great.
>
>> -- Steve
> Sebastian


Hi Steve, Sebastian,

Thanks for the review!

I came across this issue while working on the BPF side. In bpf_ringbuf,
the irq_work is embedded in struct bpf_ringbuf which is vmap'd — after
irq_work_sync(), the whole region is vunmap'd immediately 
(bpf_ringbuf_free).

Looking further, this pattern is actually widespread. Several other
subsystems embed irq_work in a dynamically allocated container and free
it right after irq_work_sync():

   - kernel/trace/ring_buffer.c:
   rb_free_cpu_buffer() syncs then kfree(cpu_buffer)
   ring_buffer_free() syncs then kfree(buffer)
   - drivers/gpu/drm/i915/gt/intel_breadcrumbs.c:
   intel_breadcrumbs_free() syncs then kfree(b)
   - kernel/sched/ext.c:
   scx_sched_free_rcu_work() syncs then kfree(sch)
   - kernel/irq/irq_sim.c:
   irq_domain_remove_sim() syncs then kfree(work_ctx)
   - drivers/iio/trigger/iio-trig-sysfs.c:
   iio_sysfs_trigger_destroy() syncs then kfree(t)
   - drivers/edac/igen6_edac.c:
   igen6_remove() syncs then kfree()


I agree that open-coding rcuwait internals is not ideal. I'd like to
check my understanding of the direction you're suggesting — would
something like the following be on the right track?

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();
'''

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();
}


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

Thanks,
Jiayuan