Re: [PATCH v9 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:
- [High] An existing BCM RX filter silently fails to re-register if its bound device is unregistered and later replaced, or if the socket is bound to 'any' interface and the device is recreated.
--

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

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

This commit fixes a missing device refcount issue in bcm.c when unregistering
a CAN filter. It introduces a netdevice_tracker to hold a reference on
op->rx_reg_dev from filter registration until unregistration, preventing
lookups on a stale device index.

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

> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 6668c1c25d923..61725c20f087e 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c

[ ... ]

> @@ -1464,7 +1463,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;

[Severity: High]
This isn't a bug introduced by this patch, but is there a scenario where an
existing BCM RX filter silently fails to re-register?

If a BCM socket is bound to interface 0, an RX_SETUP creates an operation
for a specific interface and sets op->rx_reg_dev. If that interface is then
unregistered, bcm_notify() unregisters the filter and clears op->rx_reg_dev
but leaves the operation in bo->rx_ops.

When the device is recreated and another RX_SETUP is sent for the same id
and ifindex, bcm_find_op() in bcm_rx_setup() finds the existing operation.
Because it is found, do_rx_register remains 0. The setup then returns
success but bypasses the can_rx_register() phase entirely.

Does this sequence result in the filter remaining inactive on the newly
created device and silently dropping expected CAN frames?

> +				/* 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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-bcm_fixes-v9-0-6eebb78ccfb2@hartkopp.net?part=8