From: <gregkh@linuxfoundation.org>
To: socketcan@hartkopp.net,mkl@pengutronix.de
Cc: <stable@vger.kernel.org>
Subject: FAILED: patch "[PATCH] can: isotp: fix use-after-free race with concurrent" failed to apply to 6.6-stable tree
Date: Mon, 20 Jul 2026 17:11:53 +0200 [thread overview]
Message-ID: <2026072053-spoils-overlying-285a@gregkh> (raw)
The patch below does not apply to the 6.6-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.6.y
git checkout FETCH_HEAD
git cherry-pick -x 20bab8b88baac140ca3701116e1d486c7f51e311
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2026072053-spoils-overlying-285a@gregkh' --subject-prefix 'PATCH 6.6.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 20bab8b88baac140ca3701116e1d486c7f51e311 Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Sun, 12 Jul 2026 19:59:42 +0200
Subject: [PATCH] can: isotp: fix use-after-free race with concurrent
NETDEV_UNREGISTER
isotp_release() looked up the bound network device via dev_get_by_index()
using the stored ifindex. During device unregistration the device is
unlisted from the ifindex hash before the NETDEV_UNREGISTER notifier
chain runs, so a concurrent isotp_release() could find no device, skip
can_rx_unregister() entirely, and still proceed to free the socket.
Since isotp_release() had already removed itself from the isotp
notifier list at that point, isotp_notify() would never get a chance to
clean up either, leaving a stale CAN filter that keeps pointing at the
freed socket.
Fix this the same way raw.c already does: hold a tracked reference to
the bound net_device in the socket (so->dev/so->dev_tracker) from
bind() onward instead of re-resolving it from the ifindex, and
serialize bind()/release() with rtnl_lock() so that so->dev is always
consistent with what the NETDEV_UNREGISTER notifier sees. so->dev
stays valid regardless of ifindex-hash unlisting, and is only ever
cleared by whichever of isotp_release()/isotp_notify() gets there
first, so the filter is always removed exactly once.
isotp_bind() now rejects a (re)bind with -EAGAIN while so->[tx|rx].state
isn't ISOTP_IDLE yet, so a timer left running by a prior
NETDEV_UNREGISTER can't act on a newly bound so->ifindex. Both checks
share the same lock_sock() section, so there is no window in which a
concurrent isotp_notify() clearing so->bound could be missed.
Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260707101420.47F261F000E9@smtp.kernel.org/
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260712-isotp-fixes-v10-2-793a1b1ce17f@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
diff --git a/net/can/isotp.c b/net/can/isotp.c
index d30937345bcd..44c044eb83e1 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -152,6 +152,8 @@ struct isotp_sock {
struct sock sk;
int bound;
int ifindex;
+ struct net_device *dev;
+ netdevice_tracker dev_tracker;
canid_t txid;
canid_t rxid;
ktime_t tx_gap;
@@ -978,6 +980,14 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
goto err_event_drop;
}
+ /* so->bound is only checked once above - a wakeup may have
+ * unbound/rebound the socket meanwhile, so re-validate it
+ */
+ if (!so->bound) {
+ err = -EADDRNOTAVAIL;
+ goto err_out_drop;
+ }
+
/* PDU size > default => try max_pdu_size */
if (size > so->tx.buflen && so->tx.buflen < max_pdu_size) {
u8 *newbuf = kmalloc(max_pdu_size, GFP_KERNEL);
@@ -1219,28 +1229,30 @@ static int isotp_release(struct socket *sock)
list_del(&so->notifier);
spin_unlock(&isotp_notifier_lock);
+ rtnl_lock();
lock_sock(sk);
- /* remove current filters & unregister */
- if (so->bound) {
- if (so->ifindex) {
- struct net_device *dev;
+ /* remove current filters & unregister
+ * tracked reference so->dev is taken at bind() time with rtnl_lock
+ */
+ if (so->bound && so->dev) {
+ if (isotp_register_rxid(so))
+ can_rx_unregister(net, so->dev, so->rxid,
+ SINGLE_MASK(so->rxid),
+ isotp_rcv, sk);
- dev = dev_get_by_index(net, so->ifindex);
- if (dev) {
- if (isotp_register_rxid(so))
- can_rx_unregister(net, dev, so->rxid,
- SINGLE_MASK(so->rxid),
- isotp_rcv, sk);
-
- can_rx_unregister(net, dev, so->txid,
- SINGLE_MASK(so->txid),
- isotp_rcv_echo, sk);
- dev_put(dev);
- }
- }
+ can_rx_unregister(net, so->dev, so->txid,
+ SINGLE_MASK(so->txid),
+ isotp_rcv_echo, sk);
+ netdev_put(so->dev, &so->dev_tracker);
}
+ so->ifindex = 0;
+ so->bound = 0;
+ so->dev = NULL;
+
+ rtnl_unlock();
+
/* Always wait for a grace period before touching the timers below.
* A concurrent NETDEV_UNREGISTER may have already unregistered our
* filters and cleared so->bound in isotp_notify() without waiting
@@ -1253,9 +1265,6 @@ static int isotp_release(struct socket *sock)
hrtimer_cancel(&so->txtimer);
hrtimer_cancel(&so->rxtimer);
- so->ifindex = 0;
- so->bound = 0;
-
sock_orphan(sk);
sock->sk = NULL;
@@ -1310,6 +1319,7 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
if (!addr->can_ifindex)
return -ENODEV;
+ rtnl_lock();
lock_sock(sk);
if (so->bound) {
@@ -1317,6 +1327,17 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
goto out;
}
+ /* A transmission or reception that outlived a previous binding
+ * (unbound by NETDEV_UNREGISTER) may still be draining; the FC/echo
+ * and RX watchdog timers bound how long this takes. Checked together
+ * with so->bound in the same lock_sock() section above, so there is
+ * no window in which a concurrent isotp_notify() could be missed.
+ */
+ if (so->tx.state != ISOTP_IDLE || so->rx.state != ISOTP_IDLE) {
+ err = -EAGAIN;
+ goto out;
+ }
+
/* ensure different CAN IDs when the rx_id is to be registered */
if (isotp_register_rxid(so) && rx_id == tx_id) {
err = -EADDRNOTAVAIL;
@@ -1329,14 +1350,12 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
goto out;
}
if (dev->type != ARPHRD_CAN) {
- dev_put(dev);
err = -ENODEV;
- goto out;
+ goto out_put_dev;
}
if (READ_ONCE(dev->mtu) < so->ll.mtu) {
- dev_put(dev);
err = -EINVAL;
- goto out;
+ goto out_put_dev;
}
if (!(dev->flags & IFF_UP))
notify_enetdown = 1;
@@ -1354,16 +1373,25 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l
can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id),
isotp_rcv_echo, sk, "isotpe", sk);
- dev_put(dev);
-
/* switch to new settings */
so->ifindex = ifindex;
so->rxid = rx_id;
so->txid = tx_id;
so->bound = 1;
+ /* bind() ok -> hold a reference for so->dev so that isotp_release()
+ * can safely reach the device later, even if a concurrent
+ * NETDEV_UNREGISTER has already unlisted it by ifindex.
+ */
+ so->dev = dev;
+ netdev_hold(so->dev, &so->dev_tracker, GFP_KERNEL);
+
+out_put_dev:
+ /* remove potential reference from dev_get_by_index() */
+ dev_put(dev);
out:
release_sock(sk);
+ rtnl_unlock();
if (notify_enetdown) {
sk->sk_err = ENETDOWN;
@@ -1566,7 +1594,7 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg,
if (!net_eq(dev_net(dev), sock_net(sk)))
return;
- if (so->ifindex != dev->ifindex)
+ if (so->dev != dev)
return;
switch (msg) {
@@ -1582,10 +1610,12 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg,
can_rx_unregister(dev_net(dev), dev, so->txid,
SINGLE_MASK(so->txid),
isotp_rcv_echo, sk);
+ netdev_put(so->dev, &so->dev_tracker);
}
so->ifindex = 0;
so->bound = 0;
+ so->dev = NULL;
release_sock(sk);
sk->sk_err = ENODEV;
@@ -1645,6 +1675,7 @@ static int isotp_init(struct sock *sk)
so->ifindex = 0;
so->bound = 0;
+ so->dev = NULL;
so->opt.flags = CAN_ISOTP_DEFAULT_FLAGS;
so->opt.ext_address = CAN_ISOTP_DEFAULT_EXT_ADDRESS;
next reply other threads:[~2026-07-20 15:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-20 15:11 gregkh [this message]
2026-07-24 8:53 ` [PATCH 1/2] can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER Oliver Hartkopp
2026-07-24 10:34 ` [PATCH 6.6.y " Oliver Hartkopp
2026-07-24 10:34 ` [PATCH 6.6.y 2/2] can: isotp: serialize TX state transitions under so->rx_lock Oliver Hartkopp
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2026072053-spoils-overlying-285a@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=mkl@pengutronix.de \
--cc=socketcan@hartkopp.net \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.