mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-07-22 02:17:36 -04:00
Code Issues Packages Projects Releases Wiki Activity

can: bcm: fix stale rx/tx ops after device removal

Browse Source 
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: ffd980f976 ("[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>
This commit is contained in:
Oliver Hartkopp
2026-07-14 18:55:31 +02:00
committed by Marc Kleine-Budde
parent d59948293e
commit 3b762c0d95
1 changed files with 44 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
net/can/bcm.c
View File

@@ -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)