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

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 c275a176e4b6 ("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: ffd980f976e7 ("[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>

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 2d9c9cd7453669731abc48c8acb1f21ef1a92836..25842061800ba1baf952593fb91d17a822579f39 100644 (file)
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -128,6 +128,7 @@ struct bcm_op {
        struct canfd_frame last_sframe;
        struct sock *sk;
        struct net_device *rx_reg_dev;
+       netdevice_tracker rx_reg_dev_tracker;
        spinlock_t bcm_tx_lock; /* protect tx data and timer updates */
        spinlock_t bcm_rx_update_lock; /* protect filter/timer data updates */
 };
@@ -937,6 +938,7 @@ static void bcm_rx_unreg(struct net_device *dev, struct bcm_op *op)
 
                /* mark as removed subscription */
                op->rx_reg_dev = NULL;
+               netdev_put(dev, &op->rx_reg_dev_tracker);
        } else
                printk(KERN_ERR "can-bcm: bcm_rx_unreg: registered device "
                       "mismatch %p %p\n", op->rx_reg_dev, dev);
@@ -967,17 +969,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,
@@ -1496,7 +1495,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
@@ -1873,16 +1882,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),