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

[Lee Jones <lee@kernel.org>]

On Wed, 20 May 2026, Oliver Hartkopp wrote:

> 
> 
> On 20.05.26 14:49, Lee Jones wrote:
> > On Wed, 20 May 2026, Lee Jones wrote:
> > 
> > > On Tue, 19 May 2026, Oliver Hartkopp wrote:
> > > 
> > > > From: Lee Jones <lee@kernel.org>
> > > > 
> > > > 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.
> > > > 
> > > > [Hartkopp] Added the setting of RX_NO_AUTOTIMER also to bcm_release() before
> > > > removing the CAN filters following the bcm_delete_rx_op() approach.
> > > > 
> > > > Additionally WRITE_ONCE()/READ_ONCE() macros have been introduced for
> > > > the changes of RX_NO_AUTOTIMER at rx op removal time to prevent a
> > > > potential code reordering of RX_NO_AUTOTIMER setting after CAN filter removal.
> > > > 
> > > > Signed-off-by: Lee Jones <lee@kernel.org>
> > > > Co-developed-by: Oliver Hartkopp <socketcan@hartkopp.net>
> > > 
> > > You did?  Can you add a note saying what you changed please?
> > 
> > FYI, did you also see the second swing I took at this:
> > 
> > https://lore.kernel.org/r/20260520080523.2513957-1-lee@kernel.org
> 
> Yes, and I answered to your patch.
> 
> Is there some lag in the e-mail communication right now?
> 
> That's why I also wondered why you sent a patch one day after my v2
> proposal.

Right.  I only saw your proposal today.

I've been working the alternative since Jakub NACKed the first submission.

-- 
Lee Jones