Re: [PATCH 1/1] can: bcm: prevent thrtimer UAF in rx path by checking RX_NO_AUTOTIMER

[Oliver Hartkopp]

On 27.04.26 14:40, Lee Jones wrote:
> On Sat, 25 Apr 2026, Oliver Hartkopp wrote:
>
> > On 24.04.26 21:08, Marc Kleine-Budde wrote:
> > > On 22.04.2026 14:55:50, Oliver Hartkopp wrote:
> > > > On 22.04.26 12:22, Lee Jones wrote:
> > > > > Commit f1b4e32aca08 ("can: bcm: use call_rcu() instead of costly
> > > > > synchronize_rcu()") removed the synchronize_rcu() call from
> > > > > bcm_delete_rx_op() and introduced the RX_NO_AUTOTIMER flag to prevent
> > > > > timers from being rearmed during deletion.  However, it only applied
> > > > > this check to op->timer via bcm_rx_starttimer().
> > > > >
> > > > > It missed the fact that op->thrtimer can also be rearmed by an
> > > > > in-flight bcm_rx_handler() (which runs as an RCU reader) via
> > > > > bcm_rx_update_and_send().  This allows op->thrtimer to be queued after
> > > > > bcm_remove_op() has already cancelled it, leading to a use-after-free
> > > > > when the timer fires on the deferred-freed struct bcm_op.
> > > > >
> > > > > Address the omission by checking the RX_NO_AUTOTIMER flag
> > > > > in bcm_rx_update_and_send() before starting op->thrtimer, effectively
> > > > > preventing it from being rearmed concurrently with teardown.
> > > > >
> > > > > Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> > > >
> > > > Many thanks for the investigation and the fix!
> > > >
> > > > Acked-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
> > >
> > > Can we add a Fixes: tag?
>
> Fixes: f1b4e32aca08 ("can: bcm: use call_rcu() instead of costly synchronize_rcu()")
>
> Do you need me to resubmit or are you okay to apply manually / with b4?
>
> > Yes, we should.
> >
> > Thanks!
> >
> > Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
>
> I'm a little confused by the SoB.  Does this mean you've applied it?

No. Marc will apply the patch.

My SoB is probably not needed in this context. But Marc was sometimes 
asking for it when I commented and discussed patches in the past.

I also thought Marc would add the Fixes tag on his own.

Best regards,
Oliver