* FAILED: patch "[PATCH] can: bcm: fix stale rx/tx ops after device removal" failed to apply to 6.1-stable tree
@ 2026-07-20 15:14 gregkh
2026-07-24 8:38 ` [PATCH 6/8] can: bcm: fix stale rx/tx ops after device removal Oliver Hartkopp
0 siblings, 1 reply; 2+ messages in thread
From: gregkh @ 2026-07-20 15:14 UTC (permalink / raw)
To: socketcan, mkl; +Cc: stable
The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 3b762c0d950383ab7a002686c9136b9aa55d2d70
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2026072009-earache-snowboard-eca7@gregkh' --subject-prefix 'PATCH 6.1.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 3b762c0d950383ab7a002686c9136b9aa55d2d70 Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Tue, 14 Jul 2026 18:55:31 +0200
Subject: [PATCH] can: bcm: fix stale rx/tx ops after device removal
RX: an RX_SETUP update(!) for an existing op skipped can_rx_register()
unconditionally, even when a concurrent NETDEV_UNREGISTER had already
torn down its registration (op->rx_reg_dev == NULL). This silently
did not re-enable frame delivery for that updated filter. bcm_rx_setup()
now re-registers in that case, while leaving rx_ops with ifindex = 0
(all CAN devices) which never carry a tracked rx_reg_dev registered as-is.
TX: bcm_notify() only handled bo->rx_ops on NETDEV_UNREGISTER, leaving
tx_ops with an active cyclic transmission re-arming its hrtimer
indefinitely to execute bcm_tx_timeout_handler(). Cancelling the hrtimer
prevents the runaway timer and any injection into a later reused ifindex,
since nothing else calls bcm_can_tx() for the op until an explicit
TX_SETUP update re-arms it.
Unlike bcm_rx_unreg(), which clears the tracked rx_reg_dev for rx_ops,
the ifindex is intentionally left unchanged for tx_ops. bcm_tx_setup()
always rejects ifindex 0, so clearing it would strand the op: neither a
later TX_SETUP (bcm_find_op()) nor TX_DELETE (bcm_delete_tx_op()) could
ever find it again, since both require an exact ifindex match.
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260708094536.DDF821F00A3A@smtp.kernel.org/
Closes: https://lore.kernel.org/linux-can/20260708154039.347ED1F000E9@smtp.kernel.org/
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-9-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 25842061800b..a53dba6ab8b8 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1287,6 +1287,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op;
int do_rx_register;
+ int new_op = 0;
int err = 0;
if ((msg_head->flags & RX_FILTER_ID) || (!(msg_head->nframes))) {
@@ -1371,8 +1372,15 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* free temporary frames / kfree(NULL) is safe */
kfree(new_frames);
- /* Only an update -> do not call can_rx_register() */
- do_rx_register = 0;
+ /* Don't register a new CAN filter for the rx_op update unless
+ * a concurrent NETDEV_UNREGISTER notifier already tore down
+ * the previous registration. In this case the receiver needs
+ * to be re-registered here so that this update doesn't
+ * silently stop delivering frames for the given ifindex.
+ * Ops with ifindex = 0 (all CAN interfaces) never carry a
+ * tracked rx_reg_dev and stay registered as-is.
+ */
+ do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0;
} else {
/* insert new BCM operation for the given can_id */
@@ -1439,6 +1447,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* call can_rx_register() */
do_rx_register = 1;
+ new_op = 1;
} /* if ((op = bcm_find_op(&bo->rx_ops, msg_head->can_id, ifindex))) */
@@ -1452,7 +1461,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (op->flags & SETTIMER) {
/* set timers (locked) for newly created op */
- if (do_rx_register) {
+ if (new_op) {
spin_lock_bh(&op->bcm_rx_update_lock);
op->ival1 = msg_head->ival1;
op->ival2 = msg_head->ival2;
@@ -1482,7 +1491,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
HRTIMER_MODE_REL_SOFT);
}
- /* now we can register for can_ids, if we added a new bcm_op */
+ /* now we can register for can_ids, if we added a new bcm_op
+ * or need to re-register after a NETDEV_UNREGISTER tore down
+ * the previous registration of an existing op
+ */
if (do_rx_register) {
if (ifindex) {
struct net_device *dev;
@@ -1514,18 +1526,32 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
err = -ENODEV;
}
- } else
+ } else {
err = can_rx_register(sock_net(sk), NULL, op->can_id,
REGMASK(op->can_id),
bcm_rx_handler, op, "bcm", sk);
+ }
+
if (err) {
- /* this bcm rx op is broken -> remove it */
- bcm_remove_op(op);
+ /* newly created bcm rx op is broken -> remove it */
+ if (new_op) {
+ bcm_remove_op(op);
+ return err;
+ }
+
+ /* an existing op just stays unregistered.
+ * Cancel op->timer and (defensively) op->thrtimer.
+ * Other settings can't be reached until the next
+ * successful RX_SETUP.
+ */
+ hrtimer_cancel(&op->timer);
+ hrtimer_cancel(&op->thrtimer);
return err;
}
- /* add this bcm_op to the list of the rx_ops */
- list_add_rcu(&op->list, &bo->rx_ops);
+ /* add a new bcm_op to the list of the rx_ops */
+ if (new_op)
+ list_add_rcu(&op->list, &bo->rx_ops);
}
return msg_head->nframes * op->cfsiz + MHSIZ;
@@ -1745,11 +1771,19 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
case NETDEV_UNREGISTER:
lock_sock(sk);
- /* remove device specific receive entries */
+ /* rx_ops: remove device specific receive entries */
list_for_each_entry(op, &bo->rx_ops, list)
if (op->rx_reg_dev == dev)
bcm_rx_unreg(dev, op);
+ /* tx_ops: stop device specific cyclic transmissions on the
+ * vanishing ifindex. 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)
+ hrtimer_cancel(&op->timer);
+
/* remove device reference, if this is our bound device */
if (bo->bound && bo->ifindex == dev->ifindex) {
#if IS_ENABLED(CONFIG_PROC_FS)
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH 6/8] can: bcm: fix stale rx/tx ops after device removal
2026-07-20 15:14 FAILED: patch "[PATCH] can: bcm: fix stale rx/tx ops after device removal" failed to apply to 6.1-stable tree gregkh
@ 2026-07-24 8:38 ` Oliver Hartkopp
0 siblings, 0 replies; 2+ messages in thread
From: Oliver Hartkopp @ 2026-07-24 8:38 UTC (permalink / raw)
To: stable; +Cc: Oliver Hartkopp, Marc Kleine-Budde
commit 3b762c0d950383ab7a002686c9136b9aa55d2d70 upstream.
RX: an RX_SETUP update(!) for an existing op skipped can_rx_register()
unconditionally, even when a concurrent NETDEV_UNREGISTER had already
torn down its registration (op->rx_reg_dev == NULL). This silently
did not re-enable frame delivery for that updated filter. bcm_rx_setup()
now re-registers in that case, while leaving rx_ops with ifindex = 0
(all CAN devices) which never carry a tracked rx_reg_dev registered as-is.
TX: bcm_notify() only handled bo->rx_ops on NETDEV_UNREGISTER, leaving
tx_ops with an active cyclic transmission re-arming its hrtimer
indefinitely to execute bcm_tx_timeout_handler(). Cancelling the hrtimer
prevents the runaway timer and any injection into a later reused ifindex,
since nothing else calls bcm_can_tx() for the op until an explicit
TX_SETUP update re-arms it.
Unlike bcm_rx_unreg(), which clears the tracked rx_reg_dev for rx_ops,
the ifindex is intentionally left unchanged for tx_ops. bcm_tx_setup()
always rejects ifindex 0, so clearing it would strand the op: neither a
later TX_SETUP (bcm_find_op()) nor TX_DELETE (bcm_delete_tx_op()) could
ever find it again, since both require an exact ifindex match.
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260708094536.DDF821F00A3A@smtp.kernel.org/
Closes: https://lore.kernel.org/linux-can/20260708154039.347ED1F000E9@smtp.kernel.org/
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-9-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
net/can/bcm.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 05fad88..c24fbbf 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1239,6 +1239,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op;
int do_rx_register;
+ int new_op = 0;
int err = 0;
if ((msg_head->flags & RX_FILTER_ID) || (!(msg_head->nframes))) {
@@ -1323,8 +1324,15 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* free temporary frames / kfree(NULL) is safe */
kfree(new_frames);
- /* Only an update -> do not call can_rx_register() */
- do_rx_register = 0;
+ /* Don't register a new CAN filter for the rx_op update unless
+ * a concurrent NETDEV_UNREGISTER notifier already tore down
+ * the previous registration. In this case the receiver needs
+ * to be re-registered here so that this update doesn't
+ * silently stop delivering frames for the given ifindex.
+ * Ops with ifindex = 0 (all CAN interfaces) never carry a
+ * tracked rx_reg_dev and stay registered as-is.
+ */
+ do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0;
} else {
/* insert new BCM operation for the given can_id */
@@ -1394,6 +1402,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* call can_rx_register() */
do_rx_register = 1;
+ new_op = 1;
} /* if ((op = bcm_find_op(&bo->rx_ops, msg_head->can_id, ifindex))) */
@@ -1407,7 +1416,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (op->flags & SETTIMER) {
/* set timers (locked) for newly created op */
- if (do_rx_register) {
+ if (new_op) {
spin_lock_bh(&op->bcm_rx_update_lock);
op->ival1 = msg_head->ival1;
op->ival2 = msg_head->ival2;
@@ -1437,7 +1446,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
HRTIMER_MODE_REL_SOFT);
}
- /* now we can register for can_ids, if we added a new bcm_op */
+ /* now we can register for can_ids, if we added a new bcm_op
+ * or need to re-register after a NETDEV_UNREGISTER tore down
+ * the previous registration of an existing op
+ */
if (do_rx_register) {
if (ifindex) {
struct net_device *dev;
@@ -1467,18 +1479,32 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
err = -ENODEV;
}
- } else
+ } else {
err = can_rx_register(sock_net(sk), NULL, op->can_id,
REGMASK(op->can_id),
bcm_rx_handler, op, "bcm", sk);
+ }
+
if (err) {
- /* this bcm rx op is broken -> remove it */
- bcm_remove_op(op);
+ /* newly created bcm rx op is broken -> remove it */
+ if (new_op) {
+ bcm_remove_op(op);
+ return err;
+ }
+
+ /* an existing op just stays unregistered.
+ * Cancel op->timer and (defensively) op->thrtimer.
+ * Other settings can't be reached until the next
+ * successful RX_SETUP.
+ */
+ hrtimer_cancel(&op->timer);
+ hrtimer_cancel(&op->thrtimer);
return err;
}
- /* add this bcm_op to the list of the rx_ops */
- list_add_rcu(&op->list, &bo->rx_ops);
+ /* add a new bcm_op to the list of the rx_ops */
+ if (new_op)
+ list_add_rcu(&op->list, &bo->rx_ops);
}
return msg_head->nframes * op->cfsiz + MHSIZ;
@@ -1694,11 +1720,19 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
case NETDEV_UNREGISTER:
lock_sock(sk);
- /* remove device specific receive entries */
+ /* rx_ops: remove device specific receive entries */
list_for_each_entry(op, &bo->rx_ops, list)
if (op->rx_reg_dev == dev)
bcm_rx_unreg(dev, op);
+ /* tx_ops: stop device specific cyclic transmissions on the
+ * vanishing ifindex. 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)
+ hrtimer_cancel(&op->timer);
+
/* remove device reference, if this is our bound device */
if (bo->bound && bo->ifindex == dev->ifindex) {
#if IS_ENABLED(CONFIG_PROC_FS)
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-24 8:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 15:14 FAILED: patch "[PATCH] can: bcm: fix stale rx/tx ops after device removal" failed to apply to 6.1-stable tree gregkh
2026-07-24 8:38 ` [PATCH 6/8] can: bcm: fix stale rx/tx ops after device removal Oliver Hartkopp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox