All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: remove CAN filters independent from namespace
@ 2026-09-02 18:29 Oliver Hartkopp
  2026-09-02 18:52 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2026-09-02 18:29 UTC (permalink / raw)
  To: linux-can; +Cc: Oliver Hartkopp, Norbert Szetei, stable

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 in
the CAN protocols netdev notifiers therefore led to skipping the required
removal of the CAN filters from the (namespace changed) CAN devices.

This patch removes the namespace equality check in the netdev notifiers for
BCM, ISOTP and RAW sockets. 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 original socket's bound device pointer.

In bcm.c netdevice comparisons were performed by checking the interface
index (bo->ifindex and op->ifindex) which is not namespace-safe either.
Introduce tracked netdevice pointers (bo->dev and op->tx_dev) for these
referenced devices to enable namespace-save device comparisons.

In isotp.c the two missing can_rx_unregister() calling sites are converted
to use dev_net(dev) instead of sock_net(sk) to get the correct namespace.

Fixes: 8e8cda6d737d ("can: initial support for network namespaces")
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   | 52 ++++++++++++++++++++++++++++++++++++++++++-------
 net/can/isotp.c |  7 ++-----
 net/can/raw.c   |  3 ---
 3 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 3d637a1e0ac1..42851d37cc9a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -128,18 +128,22 @@ struct bcm_op {
 	struct canfd_frame sframe;
 	struct canfd_frame last_sframe;
 	struct sock *sk;
 	struct net_device *rx_reg_dev;
 	netdevice_tracker rx_reg_dev_tracker;
+	struct net_device *tx_dev;
+	netdevice_tracker tx_dev_tracker;
 	spinlock_t bcm_tx_lock; /* protect tx data and timer updates */
 	spinlock_t bcm_rx_update_lock; /* protect filter/timer data updates */
 };
 
 struct bcm_sock {
 	struct sock sk;
 	int bound;
 	int ifindex;
+	struct net_device *dev;
+	netdevice_tracker dev_tracker;
 	struct list_head notifier;
 	struct list_head rx_ops;
 	struct list_head tx_ops;
 	unsigned long dropped_usr_msgs;
 	struct proc_dir_entry *bcm_proc_read;
@@ -933,10 +937,13 @@ static void bcm_free_op_work(struct work_struct *work)
 		kfree(op->frames);
 
 	if ((op->last_frames) && (op->last_frames != &op->last_sframe))
 		kfree(op->last_frames);
 
+	if (op->tx_dev)
+		netdev_put(op->tx_dev, &op->tx_dev_tracker);
+
 	/* the last possible access to op->timer/op->thrtimer has now
 	 * happened above via hrtimer_cancel() - op->sk is no longer
 	 * needed by any pending timer callback, so drop our reference
 	 */
 	sock_put(op->sk);
@@ -1072,10 +1079,11 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 			int ifindex, struct sock *sk)
 {
 	struct bcm_sock *bo = bcm_sk(sk);
 	struct bcm_op *op;
 	struct canfd_frame *cf;
+	struct net_device *tx_dev;
 	bool add_op_to_list = false;
 	unsigned int i;
 	int err;
 
 	/* we need a real device to send frames */
@@ -1103,10 +1111,25 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		 * therefore (complexity / locking) it is not supported.
 		 */
 		if (msg_head->nframes > op->nframes)
 			return -E2BIG;
 
+		/* Re-resolve and re-hold the target device if a concurrent
+		 * NETDEV_UNREGISTER already cleared it (see bcm_notify()).
+		 * op->ifindex is unchanged - bcm_find_op() above only
+		 * matches ops with this exact ifindex.
+		 */
+		if (!op->tx_dev) {
+			tx_dev = dev_get_by_index(sock_net(sk), ifindex);
+			if (tx_dev) {
+				op->tx_dev = tx_dev;
+				netdev_hold(tx_dev, &op->tx_dev_tracker,
+					    GFP_KERNEL);
+				dev_put(tx_dev);
+			}
+		}
+
 		/* get new CAN frames content into a staging buffer before
 		 * locking: validate and normalize the frames there so that
 		 * bcm_can_tx() / bcm_tx_timeout_handler() never observe a
 		 * partially updated or unvalidated frame in op->frames
 		 */
@@ -1221,10 +1244,16 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 
 		/* bcm_can_tx / bcm_tx_timeout_handler needs this */
 		op->sk = sk;
 		sock_hold(sk);
 		op->ifindex = ifindex;
+		tx_dev = dev_get_by_index(sock_net(sk), ifindex);
+		if (tx_dev) {
+			op->tx_dev = tx_dev;
+			netdev_hold(tx_dev, &op->tx_dev_tracker, GFP_KERNEL);
+			dev_put(tx_dev);
+		}
 
 		/* initialize uninitialized (kzalloc) structure */
 		hrtimer_setup(&op->timer, bcm_tx_timeout_handler, CLOCK_MONOTONIC,
 			      HRTIMER_MODE_REL_SOFT);
 
@@ -1792,13 +1821,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);
 
@@ -1817,19 +1843,22 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
 				spin_unlock_bh(&op->bcm_rx_update_lock);
 			}
 		}
 
 		/* tx_ops: stop device specific cyclic transmissions on the
-		 * vanishing ifindex. Cancelling the timer is enough to stop
+		 * vanishing device. Cancelling the timer is enough to stop
 		 * cyclic bcm_can_tx() calls as there is no re-arming.
 		 */
 		list_for_each_entry(op, &bo->tx_ops, list)
-			if (op->ifindex == dev->ifindex)
+			if (op->tx_dev == dev) {
 				hrtimer_cancel(&op->timer);
+				netdev_put(op->tx_dev, &op->tx_dev_tracker);
+				op->tx_dev = NULL;
+			}
 
 		/* remove device reference, if this is our bound device */
-		if (bo->bound && bo->ifindex == dev->ifindex) {
+		if (bo->bound && bo->dev == dev) {
 #if IS_ENABLED(CONFIG_PROC_FS)
 			if (sock_net(sk)->can.bcmproc_dir && bo->bcm_proc_read) {
 				remove_proc_entry(bo->procname, sock_net(sk)->can.bcmproc_dir);
 				bo->bcm_proc_read = NULL;
 			}
@@ -1839,10 +1868,12 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
 			 * accessed under lock_sock() so it needs no
 			 * annotation.
 			 */
 			WRITE_ONCE(bo->bound, 0);
 			bo->ifindex = 0;
+			netdev_put(bo->dev, &bo->dev_tracker);
+			bo->dev = NULL;
 			notify_enodev = 1;
 		}
 
 		release_sock(sk);
 
@@ -1852,11 +1883,11 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
 				sk_error_report(sk);
 		}
 		break;
 
 	case NETDEV_DOWN:
-		if (bo->bound && bo->ifindex == dev->ifindex) {
+		if (bo->bound && bo->dev == dev) {
 			sk->sk_err = ENETDOWN;
 			if (!sock_flag(sk, SOCK_DEAD))
 				sk_error_report(sk);
 		}
 	}
@@ -1982,10 +2013,14 @@ static int bcm_release(struct socket *sock)
 
 	/* remove device reference */
 	if (bo->bound) {
 		WRITE_ONCE(bo->bound, 0);
 		bo->ifindex = 0;
+		if (bo->dev) {
+			netdev_put(bo->dev, &bo->dev_tracker);
+			bo->dev = NULL;
+		}
 	}
 
 	sock_orphan(sk);
 	sock->sk = NULL;
 
@@ -2029,15 +2064,18 @@ static int bcm_connect(struct socket *sock, struct sockaddr_unsized *uaddr, int
 			ret = -ENODEV;
 			goto fail;
 		}
 
 		bo->ifindex = dev->ifindex;
+		bo->dev = dev;
+		netdev_hold(dev, &bo->dev_tracker, GFP_KERNEL);
 		dev_put(dev);
 
 	} else {
 		/* no interface reference for ifindex = 0 ('any' CAN device) */
 		bo->ifindex = 0;
+		bo->dev = NULL;
 	}
 
 #if IS_ENABLED(CONFIG_PROC_FS)
 	if (net->can.bcmproc_dir) {
 		/* unique socket address as filename */
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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] can: remove CAN filters independent from namespace
  2026-09-02 18:29 [PATCH] can: remove CAN filters independent from namespace Oliver Hartkopp
@ 2026-09-02 18:52 ` sashiko-bot
  2026-09-03  6:57   ` Oliver Hartkopp
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-09-02 18:52 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Marc Kleine-Budde, Vincent Mailhol, linux-can, Oliver Hartkopp,
	Oleksij Rempel

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Permanent struct net_device reference leak on error path in bcm_connect().
- [Medium] Cross-namespace data corruption in BCM ANYDEV receive operations.
- [Medium] Timer cancellation regression leading to leaked BCM TX timers.
- [Medium] Per-namespace CAN receive list statistics leak in isotp_release().
--

commit 510b2a0288244edb9bcc3905c8c1576ae80db01f
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: remove CAN filters independent from namespace

This patch removes the namespace equality check in the netdev notifiers for
BCM, ISOTP and RAW sockets to ensure CAN filters are removed even when the
device's namespace changes. It also introduces tracked netdevice pointers
for BCM and converts isotp.c to use the device's namespace during
unregistration.

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..42851d37cc9ae 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
[ ... ]
> @@ -1223,6 +1246,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>  		op->sk = sk;
>  		sock_hold(sk);
>  		op->ifindex = ifindex;
> +		tx_dev = dev_get_by_index(sock_net(sk), ifindex);
> +		if (tx_dev) {
> +			op->tx_dev = tx_dev;
> +			netdev_hold(tx_dev, &op->tx_dev_tracker, GFP_KERNEL);
> +			dev_put(tx_dev);
> +		}
>  
>  		/* initialize uninitialized (kzalloc) structure */
>  		hrtimer_setup(&op->timer, bcm_tx_timeout_handler, CLOCK_MONOTONIC,

[Severity: Medium]
If dev_get_by_index() returns NULL here because the interface does not exist
yet, op->tx_dev remains NULL but the timer is still armed below.

If the device is created later, bcm_can_tx() dynamically resolves it and
succeeds. However, if the device is then unregistered, bcm_notify() skips
cancelling the timer because it strictly checks op->tx_dev == dev:

    list_for_each_entry(op, &bo->tx_ops, list)
        if (op->tx_dev == dev) {
            hrtimer_cancel(&op->timer);
            ...
        }

Will this leave the hrtimer permanently leaked and firing indefinitely,
repeatedly failing device lookups?

[ ... ]
> @@ -1794,9 +1823,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;
> -
>  	switch (msg) {
>  
>  	case NETDEV_UNREGISTER:

[Severity: Medium]
Since this namespace check was removed, bcm_notify() now processes events
across all network namespaces.

For ANYDEV sockets, the code later in this function handles NETDEV_UNREGISTER
like this:

    if (!op->ifindex) {
        spin_lock_bh(&op->bcm_rx_update_lock);
        if (op->if_detected == dev->ifindex)
            op->if_detected = 0;
        spin_unlock_bh(&op->bcm_rx_update_lock);
    }

Because ifindexes are only unique per-namespace, deleting a CAN interface
in one namespace triggers this globally.

Can this cause an ifindex collision that spuriously clears the if_detected
state of an ANYDEV socket operating in a completely different namespace,
causing it to drop active frame assembly or mismanage timers?

[ ... ]
> @@ -2031,11 +2066,14 @@ static int bcm_connect(struct socket *sock, struct sockaddr_unsized *uaddr, int
>  		}
>  
>  		bo->ifindex = dev->ifindex;
> +		bo->dev = dev;
> +		netdev_hold(dev, &bo->dev_tracker, GFP_KERNEL);
>  		dev_put(dev);
>  
>  	} else {
>  		/* no interface reference for ifindex = 0 ('any' CAN device) */
>  		bo->ifindex = 0;
> +		bo->dev = NULL;
>  	}

[Severity: High]
Does this error path leak the acquired net_device reference?

If the subsequent proc_create_net_single() allocation fails, the function
jumps to the fail label and returns without calling netdev_put():

    if (!bo->bcm_proc_read) {
        ret = -ENOMEM;
        goto fail;
    }

The cleanup also skips setting WRITE_ONCE(bo->bound, 1). When the socket
is later closed, bcm_release() skips the release because bo->bound is 0:

    if (bo->bound) {
        WRITE_ONCE(bo->bound, 0);
        bo->ifindex = 0;
        if (bo->dev) {
            netdev_put(bo->dev, &bo->dev_tracker);
            bo->dev = NULL;
        }
    }

Could this permanently leak the net_device and tracker, preventing the
interface from ever being cleanly unregistered?

> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 155530aedce2e..0835a4758a72f 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1492,11 +1492,11 @@ static int isotp_release(struct socket *sock)
>  	 */
>  	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);

[Severity: Medium]
Does this leak the per-namespace receive list statistics in the socket's
original namespace?

If the device was moved to a different network namespace, this now passes
the new namespace (dev_net(so->dev)) to can_rx_unregister().

When can_rx_unregister() runs, it decrements the statistic in the
provided namespace:

    if (rcv_lists_stats->rcv_entries > 0)
        rcv_lists_stats->rcv_entries--;

Because can_rx_register() incremented the counter in the socket's original
namespace (sock_net(sk)), does decrementing it in the new namespace permanently
leak the count in the old one, while inappropriately decrementing the new one?

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] can: remove CAN filters independent from namespace
  2026-09-02 18:52 ` sashiko-bot
@ 2026-09-03  6:57   ` Oliver Hartkopp
  0 siblings, 0 replies; 3+ messages in thread
From: Oliver Hartkopp @ 2026-09-03  6:57 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Marc Kleine-Budde, Vincent Mailhol, linux-can, Oleksij Rempel



On 02.09.26 20:52, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Permanent struct net_device reference leak on error path in bcm_connect().
> - [Medium] Cross-namespace data corruption in BCM ANYDEV receive operations.
> - [Medium] Timer cancellation regression leading to leaked BCM TX timers.
> - [Medium] Per-namespace CAN receive list statistics leak in isotp_release().
> --
> 
> commit 510b2a0288244edb9bcc3905c8c1576ae80db01f
> Author: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> can: remove CAN filters independent from namespace
> 
> This patch removes the namespace equality check in the netdev notifiers for
> BCM, ISOTP and RAW sockets to ensure CAN filters are removed even when the
> device's namespace changes. It also introduces tracked netdevice pointers
> for BCM and converts isotp.c to use the device's namespace during
> unregistration.
> 
> 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..42851d37cc9ae 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
> [ ... ]
>> @@ -1223,6 +1246,12 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>>   		op->sk = sk;
>>   		sock_hold(sk);
>>   		op->ifindex = ifindex;
>> +		tx_dev = dev_get_by_index(sock_net(sk), ifindex);
>> +		if (tx_dev) {
>> +			op->tx_dev = tx_dev;
>> +			netdev_hold(tx_dev, &op->tx_dev_tracker, GFP_KERNEL);
>> +			dev_put(tx_dev);
>> +		}
>>   
>>   		/* initialize uninitialized (kzalloc) structure */
>>   		hrtimer_setup(&op->timer, bcm_tx_timeout_handler, CLOCK_MONOTONIC,
> 
> [Severity: Medium]
> If dev_get_by_index() returns NULL here because the interface does not exist
> yet, op->tx_dev remains NULL but the timer is still armed below.
> 
> If the device is created later, bcm_can_tx() dynamically resolves it and
> succeeds. However, if the device is then unregistered, bcm_notify() skips
> cancelling the timer because it strictly checks op->tx_dev == dev:
> 
>      list_for_each_entry(op, &bo->tx_ops, list)
>          if (op->tx_dev == dev) {
>              hrtimer_cancel(&op->timer);
>              ...
>          }
> 
> Will this leave the hrtimer permanently leaked and firing indefinitely,
> repeatedly failing device lookups?
> 
> [ ... ]

Yes. v2 adds an error handling for dev_get_by_index() calls for tx_dev 
in bcm_tx_setup()

>> @@ -1794,9 +1823,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;
>> -
>>   	switch (msg) {
>>   
>>   	case NETDEV_UNREGISTER:
> 
> [Severity: Medium]
> Since this namespace check was removed, bcm_notify() now processes events
> across all network namespaces.
> 
> For ANYDEV sockets, the code later in this function handles NETDEV_UNREGISTER
> like this:
> 
>      if (!op->ifindex) {
>          spin_lock_bh(&op->bcm_rx_update_lock);
>          if (op->if_detected == dev->ifindex)
>              op->if_detected = 0;
>          spin_unlock_bh(&op->bcm_rx_update_lock);
>      }
> 
> Because ifindexes are only unique per-namespace, deleting a CAN interface
> in one namespace triggers this globally.
> 
> Can this cause an ifindex collision that spuriously clears the if_detected
> state of an ANYDEV socket operating in a completely different namespace,
> causing it to drop active frame assembly or mismanage timers?
> 
> [ ... ]

Yes. v2 re-introduces the net_eq() check for ANYDEV case handling of 
op->ifdetected

>> @@ -2031,11 +2066,14 @@ static int bcm_connect(struct socket *sock, struct sockaddr_unsized *uaddr, int
>>   		}
>>   
>>   		bo->ifindex = dev->ifindex;
>> +		bo->dev = dev;
>> +		netdev_hold(dev, &bo->dev_tracker, GFP_KERNEL);
>>   		dev_put(dev);
>>   
>>   	} else {
>>   		/* no interface reference for ifindex = 0 ('any' CAN device) */
>>   		bo->ifindex = 0;
>> +		bo->dev = NULL;
>>   	}
> 
> [Severity: High]
> Does this error path leak the acquired net_device reference?
> 
> If the subsequent proc_create_net_single() allocation fails, the function
> jumps to the fail label and returns without calling netdev_put():
> 
>      if (!bo->bcm_proc_read) {
>          ret = -ENOMEM;
>          goto fail;
>      }
> 
> The cleanup also skips setting WRITE_ONCE(bo->bound, 1). When the socket
> is later closed, bcm_release() skips the release because bo->bound is 0:
> 
>      if (bo->bound) {
>          WRITE_ONCE(bo->bound, 0);
>          bo->ifindex = 0;
>          if (bo->dev) {
>              netdev_put(bo->dev, &bo->dev_tracker);
>              bo->dev = NULL;
>          }
>      }
> 
> Could this permanently leak the net_device and tracker, preventing the
> interface from ever being cleanly unregistered?
> 

Yes. v2 drops bo->dev reference tracking when procfs initialization fails

>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index 155530aedce2e..0835a4758a72f 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
>> @@ -1492,11 +1492,11 @@ static int isotp_release(struct socket *sock)
>>   	 */
>>   	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);
> 
> [Severity: Medium]
> Does this leak the per-namespace receive list statistics in the socket's
> original namespace?
> 
> If the device was moved to a different network namespace, this now passes
> the new namespace (dev_net(so->dev)) to can_rx_unregister().
> 
> When can_rx_unregister() runs, it decrements the statistic in the
> provided namespace:
> 
>      if (rcv_lists_stats->rcv_entries > 0)
>          rcv_lists_stats->rcv_entries--;
> 
> Because can_rx_register() incremented the counter in the socket's original
> namespace (sock_net(sk)), does decrementing it in the new namespace permanently
> leak the count in the old one, while inappropriately decrementing the new one?
> 

Yes. A cosmetic problem that can be fixed in a follow-up af_can.c patch. 
Not relevant for this stable fix.

Oliver

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-03  6:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 18:29 [PATCH] can: remove CAN filters independent from namespace Oliver Hartkopp
2026-09-02 18:52 ` sashiko-bot
2026-09-03  6:57   ` Oliver Hartkopp

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.