isotp_release() removes the socket from isotp_notifier_list before it takes rtnl_lock(). As the netdev notifier chain runs under RTNL, a socket that leaves the list in this window is skipped by isotp_notify() and has to remove its CAN filters itself. This fails when the bound device is moved to another network namespace in this window: isotp_release() passes sock_net(sk) to can_rx_unregister(), which silently returns when this namespace does not match dev_net(dev) anymore. As can_rx_register() stores rcv->sk without taking a reference, the remaining receivers point to the freed socket and travel with the device into the new namespace: BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0 Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578 can_rcv_filter+0x4af/0x8c0 can_receive+0x28d/0x3c0 can_rcv+0x2a9/0x310 __netif_receive_skb_one_core+0x21a/0x260 process_backlog+0x210/0x760 Take rtnl_lock() before removing the socket from the notifier list, so that isotp_release() and isotp_notify() can not both skip the removal. As the notifier chain is always invoked under RTNL (see ASSERT_RTNL() in call_netdevice_notifiers_info()), isotp_busy_notifier can not point to the released socket anymore. Replace the now dead busy-wait loop with a WARN_ON_ONCE(). isotp_notifier_lock is still needed, as isotp_init() adds new sockets to isotp_notifier_list without holding RTNL. Additionally use dev_net(so->dev) for the can_rx_unregister() calls, as the CAN filters live in the device's namespace. With the new locking both namespaces are equal there, but dev_net(so->dev) is stable under RTNL and always names the correct namespace. Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER") Reported-by: Norbert Szetei Link: https://lore.kernel.org/linux-can/CEA6A38A-2646-4ADA-95B4-CBAE2F301A8E@doyensec.com/ Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp --- net/can/isotp.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index f5dc9d04bd68..186daa818647 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1477,32 +1477,33 @@ static int isotp_release(struct socket *sock) WRITE_ONCE(so->rx.state, ISOTP_IDLE); /* forced SHUTDOWN may have skipped IDLE (gave up on a signal) */ wake_up_interruptible(&so->wait); + rtnl_lock(); + + /* The netdev notifier chain runs under RTNL, so isotp_notifier() can't + * be walking isotp_notifier_list and point to this socket. The spinlock + * is still needed as isotp_init() adds sockets without holding RTNL. + */ spin_lock(&isotp_notifier_lock); - while (isotp_busy_notifier == so) { - spin_unlock(&isotp_notifier_lock); - schedule_timeout_uninterruptible(1); - spin_lock(&isotp_notifier_lock); - } + WARN_ON_ONCE(isotp_busy_notifier == so); list_del(&so->notifier); spin_unlock(&isotp_notifier_lock); - rtnl_lock(); lock_sock(sk); /* remove current filters & unregister * tracked reference so->dev is taken at bind() time with rtnl_lock */ if (so->bound && so->dev) { if (isotp_register_rxid(so)) - can_rx_unregister(net, so->dev, so->rxid, + can_rx_unregister(dev_net(so->dev), so->dev, so->rxid, SINGLE_MASK(so->rxid), isotp_rcv, sk); - can_rx_unregister(net, so->dev, so->txid, + can_rx_unregister(dev_net(so->dev), so->dev, so->txid, SINGLE_MASK(so->txid), isotp_rcv_echo, sk); netdev_put(so->dev, &so->dev_tracker); } -- 2.53.0