[PATCH] can: notifiers: remove CAN filters independent from namespace

[PATCH] can: notifiers: remove CAN filters independent from namespace

[Oliver Hartkopp]

Remove the namespace equality check in the netdev notifiers for BCM,
ISOTP, and RAW sockets.

When the devices namespace is changed the socket namespace and the device
namespace might differ. The net_eq(dev_net(dev), sock_net(sk)) check
therefore skipped the removal of the CAN filters from the (ns changed)
CAN device.

Since the `struct net_device` pointer is globally unique, the notifier
should always process the unregister event and remove the CAN filters if
it matches the socket's bound device, regardless of the device's current
network namespace.

Change all can_rx_unregister() calling sites to use dev_net(dev) instead
of sock_net(sk) to retrieve the namespace (only missing in isotp.c).

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Reported-by: Norbert Szetei <norbert@doyensec.com>
Link: https://lore.kernel.org/linux-can/CEA6A38A-2646-4ADA-95B4-CBAE2F301A8E@doyensec.com/
Cc: stable@vger.kernel.org
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/bcm.c   | 3 ---
 net/can/isotp.c | 7 ++-----
 net/can/raw.c   | 3 ---
 3 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 3d637a1e0ac1..aaa5163c46cc 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1792,13 +1792,10 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
 {
 	struct sock *sk = &bo->sk;
 	struct bcm_op *op;
 	int notify_enodev = 0;
 
-	if (!net_eq(dev_net(dev), sock_net(sk)))
-		return;
-
 	switch (msg) {
 
 	case NETDEV_UNREGISTER:
 		lock_sock(sk);
 
diff --git a/net/can/isotp.c b/net/can/isotp.c
index 155530aedce2..0835a4758a72 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1490,15 +1490,15 @@ static int isotp_release(struct socket *sock)
 	/* 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);
 	}
 
@@ -1846,13 +1846,10 @@ static int isotp_getsockopt(struct socket *sock, int level, int optname,
 static void isotp_notify(struct isotp_sock *so, unsigned long msg,
 			 struct net_device *dev)
 {
 	struct sock *sk = &so->sk;
 
-	if (!net_eq(dev_net(dev), sock_net(sk)))
-		return;
-
 	if (so->dev != dev)
 		return;
 
 	switch (msg) {
 	case NETDEV_UNREGISTER:
diff --git a/net/can/raw.c b/net/can/raw.c
index 82d9c0499c95..c5596fc9aac5 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -300,13 +300,10 @@ static int raw_enable_allfilters(struct net *net, struct net_device *dev,
 static void raw_notify(struct raw_sock *ro, unsigned long msg,
 		       struct net_device *dev)
 {
 	struct sock *sk = &ro->sk;
 
-	if (!net_eq(dev_net(dev), sock_net(sk)))
-		return;
-
 	if (ro->dev != dev)
 		return;
 
 	switch (msg) {
 	case NETDEV_UNREGISTER:
-- 
2.53.0

Re: [PATCH] can: notifiers: remove CAN filters independent from namespace

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the namespace equality check in `bcm_notify` exposes BCM sockets to cross-namespace `ifindex` collisions. Unprivileged users can exploit this to disrupt BCM sockets in the host or other namespaces by spoofing `NETDEV_UNREGISTER` and `NETDEV_DOWN` events.
--

commit e5738773c45b8a30848fd2bb47977b2587f26903
Author: Oliver Hartkopp <socketcan@hartkopp.net>
can: notifiers: remove CAN filters independent from namespace
    
This commit removes the namespace equality check in the netdev notifiers
for BCM, ISOTP, and RAW CAN sockets. The goal is to ensure CAN filters
are correctly removed when a device is unregistered, even if the socket
and device network namespaces differ due to a namespace change. It also
updates the can_rx_unregister() calling sites to retrieve the namespace
from the device instead of the socket.

Link: https://lore.kernel.org/linux-can/CEA6A38A-2646-4ADA-95B4-CBAE2F301A8E@doyensec.com/

> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 3d637a1e0ac1a..aaa5163c46cc2 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -1794,9 +1794,6 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
>  	struct bcm_op *op;
>  	int notify_enodev = 0;
>  
> -	if (!net_eq(dev_net(dev), sock_net(sk)))
> -		return;
> -

[Severity: High]
With this namespace equality check removed, are BCM sockets now exposed to
cross-namespace ifindex collisions?

Because ifindex is only unique per namespace, an unprivileged user could create
a virtual CAN device in a new network namespace with an ifindex matching a
target BCM socket in another namespace.

When this spoofed device is unregistered or brought down, the events proceed
into the rest of bcm_notify() which compares the integer ifindex instead of
a device pointer:

net/can/bcm.c:bcm_notify() {
    ...
    case NETDEV_UNREGISTER:
        ...
        /* ANYDEV ops */
        if (!op->ifindex) {
            spin_lock_bh(&op->bcm_rx_update_lock);
            if (op->if_detected == dev->ifindex)
                op->if_detected = 0;
            ...
        /* tx_ops */
        list_for_each_entry(op, &bo->tx_ops, list)
            if (op->ifindex == dev->ifindex)
                hrtimer_cancel(&op->timer);
        ...
        /* bound device */
        if (bo->bound && bo->ifindex == dev->ifindex) {
            ...
            WRITE_ONCE(bo->bound, 0);
            bo->ifindex = 0;
            notify_enodev = 1;
        }
    ...
    case NETDEV_DOWN:
        if (bo->bound && bo->ifindex == dev->ifindex) {
            sk->sk_err = ENETDOWN;
            ...
}

Could this allow an attacker to falsely clear ANYDEV claims, cancel active
transmission timers, forcibly unbind the socket (ENODEV), or inject ENETDOWN
errors into victim BCM sockets across namespaces?

>  	switch (msg) {
>  
>  	case NETDEV_UNREGISTER:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831185654.16701-1-socketcan@hartkopp.net?part=1