Re: [PATCH net] can: bcm: add missing synchronize_rcu() in bcm_delete_rx_op()
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: 2026-07-27 08:57:40
Also in:
linux-can, lkml
On 27.07.26 10:37, Nguyen Le Thanh Tung wrote:
bcm_delete_rx_op(), reachable from an unprivileged user via an RX_DELETE
bcm_msg_head, unregisters the CAN rx handler and then immediately calls
bcm_remove_op(), which cancels op->timer and frees the op via call_rcu():
can_rx_unregister(..., bcm_rx_handler, op);
list_del_rcu(&op->list);
bcm_remove_op(op); /* hrtimer_cancel() + call_rcu() */
bcm_rx_handler() runs in an RCU read-side critical section and re-arms
op->timer on frame reception via bcm_rx_starttimer(). A handler that is
already in flight on another CPU when RX_DELETE is processed can be past
its RX_NO_AUTOTIMER check -- a plain, unordered u32 store set a few lines
earlier in bcm_delete_rx_op() -- and re-arm the timer after bcm_remove_op()
has cancelled it. The op is then freed after the grace period with its
hrtimer still enqueued, so the timer fires on freed memory in the timer
softirq: an out-of-bounds access that can crash the machine or disclose
adjacent kernel memory. AF_CAN BCM sockets are reachable in a user +
network namespace (e.g. over a vcan interface), so this is triggerable
by an unprivileged local user.
commit d5f9023fa61e ("can: bcm: delay release of struct bcm_op after
synchronize_rcu()") fixed exactly this race for the socket teardown path
in bcm_release(), by draining in-flight handlers with synchronize_rcu()
between can_rx_unregister() and bcm_remove_op(), but the RX_DELETE path in
bcm_delete_rx_op() was left unchanged and still has the race.
Add the same synchronize_rcu() to bcm_delete_rx_op() so that no
bcm_rx_handler() can be running -- and thus re-arm the timer -- once
bcm_remove_op() cancels it and frees the op. bcm_delete_rx_op() is only
called from bcm_sendmsg() under lock_sock() in process context, so
sleeping in synchronize_rcu() is safe here, exactly as it already is in
bcm_release().
Fixes: d5f9023fa61e ("can: bcm: delay release of struct bcm_op after synchronize_rcu()")
Signed-off-by: Nguyen Le Thanh Tung <redacted>NAK This is already fixed with https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/net/can/bcm.c?id=68973f9db76144825e4f35dfdc80fb8279eb2d57 ("can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF") without re-introducing the costly synchronize_rcu(). There are a bunch of bcm fixes that have been applied to 7.2-rc3 and are queued for several stable trees. Please re-check the current bcm.c developments and fixes. Best regards, Oliver
quoted hunk ↗ jump to hunk
--- net/can/bcm.c | 8 ++++++++ 1 file changed, 8 insertions(+)diff --git a/net/can/bcm.c b/net/can/bcm.c index 75653584f..25f868530 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c@@ -822,6 +822,14 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh, bcm_rx_handler, op); list_del_rcu(&op->list); + + /* wait for any in-flight bcm_rx_handler() to finish so a + * concurrent frame reception cannot re-arm op->timer after + * bcm_remove_op() has cancelled it and the op is freed + * (same drain as the bcm_release() teardown path). + */ + synchronize_rcu(); + bcm_remove_op(op); return 1; /* done */ }