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

commit d59948293e upstream.

sashiko-bot remarked a problem with a concurrent device unregistration
in isotp.c which also is present in the bcm.c code. A former fix for raw.c
commit c275a176e4 ("can: raw: add missing refcount for memory leak fix")
introduced a netdevice_tracker which solves the issue for bcm.c too.

bcm_release(), bcm_delete_rx_op() and bcm_notifier() relied on
dev_get_by_index(ifindex) to re-find the device for an rx_op before
unregistering its filter. If a concurrent NETDEV_UNREGISTER has already
unlisted the device from the ifindex table, that lookup fails and
can_rx_unregister() is silently skipped, leaving a stale CAN filter
pointing at the soon-to-be-freed bcm_op/socket.

Hold a netdev_hold()/netdev_put() tracked reference on op->rx_reg_dev
from the moment the rx filter is registered in bcm_rx_setup() until it
is unregistered in bcm_rx_unreg(), and use that reference directly in
bcm_release() and bcm_delete_rx_op() instead of re-looking the device
up by ifindex.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260707094716.63578-1-socketcan@hartkopp.net
Fixes: ffd980f976 ("[CAN]: Add broadcast manager (bcm) protocol")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-8-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Oliver Hartkopp
2026-08-03 11:15:11 +02:00
committed by Greg Kroah-Hartman
parent 7d966cdee0
commit 84aa480781
+24 -20
View File
@@ -889,6 +889,7 @@ static void bcm_rx_unreg(struct net_device *dev, struct bcm_op *op)
/* mark as removed subscription */
op->rx_reg_dev = NULL;
dev_put(dev);
} else
printk(KERN_ERR "can-bcm: bcm_rx_unreg: registered device "
"mismatch %p %p\n", op->rx_reg_dev, dev);
@@ -919,17 +920,14 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
* Only remove subscriptions that had not
* been removed due to NETDEV_UNREGISTER
* in bcm_notifier()
*
* op->rx_reg_dev is a tracked reference taken
* when the subscription was registered, so it
* stays valid here even if a concurrent
* NETDEV_UNREGISTER already unlisted the dev.
*/
if (op->rx_reg_dev) {
struct net_device *dev;
dev = dev_get_by_index(sock_net(op->sk),
op->ifindex);
if (dev) {
bcm_rx_unreg(dev, op);
dev_put(dev);
}
}
if (op->rx_reg_dev)
bcm_rx_unreg(op->rx_reg_dev, op);
} else
can_rx_unregister(sock_net(op->sk), NULL,
op->can_id,
@@ -1452,7 +1450,15 @@ 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 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;
dev_hold(dev);
}
dev_put(dev);
} else {
/* the requested device is gone - do not
@@ -1825,16 +1831,14 @@ static int bcm_release(struct socket *sock)
* Only remove subscriptions that had not
* been removed due to NETDEV_UNREGISTER
* in bcm_notifier()
*
* op->rx_reg_dev is a tracked reference taken
* when the subscription was registered, so it
* stays valid here even if a concurrent
* NETDEV_UNREGISTER already unlisted the device.
*/
if (op->rx_reg_dev) {
struct net_device *dev;
dev = dev_get_by_index(net, op->ifindex);
if (dev) {
bcm_rx_unreg(dev, op);
dev_put(dev);
}
}
if (op->rx_reg_dev)
bcm_rx_unreg(op->rx_reg_dev, op);
} else
can_rx_unregister(net, NULL, op->can_id,
REGMASK(op->can_id),