[PATCH] can: bcm: fix filter removal race between release and netns change

[Oliver Hartkopp <socketcan@hartkopp.net>]

bcm_release() removes the socket from bcm_notifier_list before it takes
any lock that serializes against the netdev notifier chain, which runs
under RTNL. A socket that leaves the list in this window is skipped by
bcm_notify() and has to remove its CAN filters itself.

This is the same problem as reported for isotp_release() and fixed in a
separate patch: if the bound device changes its namespace in this
window, can_rx_unregister() can see a different netns than the one
passed in by bcm_rx_unreg() and silently skips the removal. The op is
freed anyway and the remaining receiver leads to a use-after-free in
bcm_rx_handler().

Take rtnl_lock() before removing the socket from the notifier list, so
that bcm_release() and bcm_notify() can not both skip the removal and
dev_net(dev) is stable while the filters are removed. RTNL is dropped
before synchronize_rcu(). As bcm_busy_notifier can not point to the
released socket under RTNL, replace the busy-wait loop with a
WARN_ON_ONCE().

Fixes: 8e8cda6d737d ("can: initial support for network namespaces")
Link: https://lore.kernel.org/linux-can/CEA6A38A-2646-4ADA-95B4-CBAE2F301A8E@doyensec.com/
Cc: Norbert Szetei <norbert@doyensec.com>
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/bcm.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 3d637a1e0ac1..4765c14947fc 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1924,16 +1924,18 @@ static int bcm_release(struct socket *sock)
 	net = sock_net(sk);
 	bo = bcm_sk(sk);
 
 	/* remove bcm_ops, timer, rx_unregister(), etc. */
 
+	rtnl_lock();
+
+	/* The netdev notifier chain runs under RTNL, so bcm_notifier() can't
+	 * be walking bcm_notifier_list and point to this socket. The spinlock
+	 * is still needed as bcm_init() adds sockets without holding RTNL.
+	 */
 	spin_lock(&bcm_notifier_lock);
-	while (bcm_busy_notifier == bo) {
-		spin_unlock(&bcm_notifier_lock);
-		schedule_timeout_uninterruptible(1);
-		spin_lock(&bcm_notifier_lock);
-	}
+	WARN_ON_ONCE(bcm_busy_notifier == bo);
 	list_del(&bo->notifier);
 	spin_unlock(&bcm_notifier_lock);
 
 	lock_sock(sk);
 
@@ -1971,10 +1973,12 @@ static int bcm_release(struct socket *sock)
 					  REGMASK(op->can_id),
 					  bcm_rx_handler, op);
 
 	}
 
+	rtnl_unlock();
+
 	synchronize_rcu();
 
 	list_for_each_entry_safe(op, next, &bo->rx_ops, list) {
 		list_del_rcu(&op->list);
 		bcm_remove_op(op);
-- 
2.53.0