Skip to contentSkip to conversation

FAILED: patch "[PATCH] can: bcm: add missing device refcount for CAN filter removal" failed to apply to 5.10-stable tree

PATCH In review [email protected] 2 participants 5 messages Jul 20 – Jul 24

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-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 <[email protected]>.

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-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x d59948293ea34b6337ce2b5febab8510de70048c
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026072041-silica-outing-8bb2@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d59948293ea34b6337ce2b5febab8510de70048c Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <[email protected]>
Date: Tue, 14 Jul 2026 18:55:30 +0200
Subject: [PATCH] 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.

net/can/bcm.c +27 -20
index 2d9c9cd74536..25842061800b 100644
@@ -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),
maintainer
#1 lore raw

Dear Greg,

sorry for the bunch of patches the AI reports and review results created for the stable trees.

Based on the patches, the stable trees and your status mails I asked Claude to create a status table (markdown):

| # | commit | file | message | 5.10 | 5.15 | 6.1 | 6.6 | 6.12 | 6.18 | 7.1 |
|---|---|---|---|---|---|---|---|---|---|---|
| 1 | `9b1a02e0d980` | isotp.c | use unconditional synchronize_rcu() in isotp_release() | OK | OK | OK | OK | OK | OK | OK |
| 2 | `20bab8b88baa` | isotp.c | fix use-after-free race with concurrent NETDEV_UNREGISTER | FAIL | FAIL | FAIL | FAIL | FAIL | OK | OK |
| 3 | `cf070fe33bfb` | isotp.c | serialize TX state transitions under so->rx_lock | FAIL | FAIL | FAIL | FAIL | FAIL | OK | OK |
| 4 | `68973f9db761` | bcm.c | defer rx_op deallocation to workqueue to fix thrtimer UAF | FAIL | FAIL | OK | OK | OK | OK | OK |
| 5 | `d9b091d9d22f` | bcm.c | fix lockless bound/ifindex race and silent RX_SETUP failure | OK | OK | OK | OK | OK | OK | OK |
| 6 | `749179c2e25b` | bcm.c | add locking when updating filter and timer values | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 7 | `e6c24ba95fc3` | bcm.c | fix CAN frame rx/tx statistics | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 8 | `7b2c3eabc4da` | bcm.c | add missing rcu list annotations and operations | OK | OK | OK | OK | OK | OK | OK |
| 9 | `12ce799f7ab1` | bcm.c | extend bcm_tx_lock usage for data and timer updates | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 10 | `62ec41f36464` | bcm.c | validate frame length in bcm_rx_setup() for RTR replies | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 11 | `d59948293ea3` | bcm.c | add missing device refcount for CAN filter removal | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 12 | `3b762c0d9503` | bcm.c | fix stale rx/tx ops after device removal | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 13 | `58fd6cbc8541` | bcm.c | fix data race on rx_stamp/rx_ifindex in bcm_rx_handler() | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |
| 14 | `2f5976f54a04` | bcm.c | track a single source interface for ANYDEV timeout/throttle ops | FAIL | FAIL | FAIL | FAIL | FAIL | FAIL | OK |

This will take some time but I'll send a patch for each failed apply.

Best regards,
Oliver

Quoted 145 lines · On 20.07.26 17:14, [email protected] wrote:

The patch below does not apply to the 5.10-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 <[email protected]>.

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-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x d59948293ea34b6337ce2b5febab8510de70048c
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026072041-silica-outing-8bb2@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d59948293ea34b6337ce2b5febab8510de70048c Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <[email protected]>
Date: Tue, 14 Jul 2026 18:55:30 +0200
Subject: [PATCH] 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.

net/can/bcm.c +27 -20
index 2d9c9cd74536..25842061800b 100644
@@ -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),
author maintainer
#2 lore raw
Quoted 4 lines · Oliver Hartkopp

Dear Greg,

sorry for the bunch of patches the AI reports and review results created for the stable trees.

What do you mean?

Quoted 2 lines

Based on the patches, the stable trees and your status mails I asked Claude to create a status table (markdown):

So you want AI to make more work for us?

We can't read this:

Quoted 6 lines
| # | commit | file | message | 5.10 | 5.15 | 6.1 | 6.6 | 6.12 | 6.18 | 7.1
|
|---|---|---|---|---|---|---|---|---|---|---|
| 1 | `9b1a02e0d980` | isotp.c | use unconditional synchronize_rcu() in
isotp_release() | OK | OK | OK | OK | OK | OK | OK |

What does this mean?

confused,

greg k-h

maintainer
#3 lore raw

Oh I didn't want to confuse you, sorry.

Quoted 7 lines · Greg KH
Quoted 4 lines · Oliver Hartkopp

Dear Greg,

sorry for the bunch of patches the AI reports and review results created for the stable trees.

What do you mean?

Multiple issue reports reached me recently where people showed up with issues they found with the help of AI.

Quoted 4 lines
Quoted 2 lines

Based on the patches, the stable trees and your status mails I asked Claude to create a status table (markdown):

So you want AI to make more work for us?

No, we are still in charge.

Quoted 10 lines

We can't read this:

Quoted 6 lines
| # | commit | file | message | 5.10 | 5.15 | 6.1 | 6.6 | 6.12 | 6.18 | 7.1
|
|---|---|---|---|---|---|---|---|---|---|---|
| 1 | `9b1a02e0d980` | isotp.c | use unconditional synchronize_rcu() in
isotp_release() | OK | OK | OK | OK | OK | OK | OK |

What does this mean?

Sorry, it was just a table in the text markdown format (known e.g. from github) that showed which patches have been successfully applied to which stable tree. (see attached PNG)

So I was able to detect which patches need to be backported for which file and which stable tree.

With 14 patches to be applied to 7 stable trees (98 patches) I was loosing the overview. So filling your commit/FAIL mails into the AI helped to structure the plan to go.

That's all.

Best regards,
Oliver

maintainer
#4 lore raw

PATCH 6/9 can: bcm: add missing device refcount for CAN filter removal

In reply to [email protected]

commit d59948293ea34b6337ce2b5febab8510de70048c 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 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.

---
 net/can/bcm.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)
net/can/bcm.c +24 -20
index 09a6bd5..813087a 100644
@@ -884,6 +884,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);
@@ -914,17 +915,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,
@@ -1447,7 +1445,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
@@ -1820,16 +1826,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),
Signature · 1 line
2.43.0