Re: [PATCH v10 08/10] can: bcm: add missing device refcount for CAN filter removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] bcm_rx_setup() silently fails to re-register an existing CAN receive filter after a NETDEV_UNREGISTER event, causing silent packet drops.
--

--- Patch [8]: [PATCH v10 08/10] can: bcm: add missing device refcount for CAN filter removal ---
commit 1e97ac5794b00273de0d8b7a26438344b6bbef67
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: bcm: add missing device refcount for CAN filter removal

This commit fixes a concurrent device unregistration issue by introducing a
netdevice_tracker. This ensures the bcm code safely holds a reference to the
CAN interface, preventing stale CAN filters from pointing at freed structures
if the device is unlisted before the filter is removed.

Link: https://sashiko.dev/#/patchset/20260707094716.63578-1-socketcan@hartkopp.net

> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 3cc3deb98a984..5660165cdf0eb 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
[ ... ]
> @@ -1465,7 +1464,17 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>  						      bcm_rx_handler, op,
>  						      "bcm", sk);
>  
> -				op->rx_reg_dev = dev;
> +				/* keep a tracked reference so that a later
> +				 * unregister can safely reach the device even
> +				 * if a concurrent NETDEV_UNREGISTER has
> +				 * already unlisted it by ifindex
> +				 */
> +				if (!err) {
> +					op->rx_reg_dev = dev;
> +					netdev_hold(dev,
> +						    &op->rx_reg_dev_tracker,
> +						    GFP_KERNEL);
> +				}
>  				dev_put(dev);
>  			} else {
>  				/* the requested device is gone - do not

[Severity: Medium]
This isn't a bug introduced by this patch, but does bcm_rx_setup() silently
fail to re-register an existing CAN receive filter after a NETDEV_UNREGISTER
event?

If a device is unregistered, bcm_notifier() unregisters the filter and sets
op->rx_reg_dev = NULL, while keeping the op in the socket's list.

If the device is recreated with the same ifindex and the user sends another
RX_SETUP to reconfigure it, bcm_find_op() finds the existing op and
bcm_rx_setup() unconditionally forces do_rx_register to 0 for updates:

    /* Only an update -> do not call can_rx_register() */
    do_rx_register = 0;

Because of this, the RX_SETUP update succeeds from the user's perspective, but
can_rx_register() is skipped and the filter is left permanently detached,
causing silent packet drops.

Could we check if op->rx_reg_dev is NULL here to ensure we re-register the
filter when needed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-bcm_fixes-v10-0-57fff38e34ce@hartkopp.net?part=8