From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [85.215.255.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 66B343F6C39 for ; Fri, 10 Jul 2026 08:54:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=85.215.255.20 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783673654; cv=pass; b=e/DAu8N45ZxKi2/yNGtxBcD0qDx7BxMFyJ0CX2qlHHUJOu/TNT6phSj2hiEnFS0C/Z3OzE5imEnXsTaJGpY7fm0Q5CTBF2inXoPXkfDTPcbiXg7rGy10O6yxa+i7qevPaXH3LkMqPK9NAbKzod2Gy72j7/X7NGSHMt/jFaAIXqY= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783673654; c=relaxed/simple; bh=1IREWZ2p51JNnzO6MoSgtFmp+pJYtVzZqc8P7Tmxf+c=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type; b=KfTg32z0G9+U4qXK8k1z9MW1rJb7RsSDaYX8ivjAbSYbAAgsWXyn88ORUZK0mMQ9xPcTCe+mGB7evDROTKOgTRcqPgGUmSnwdOMRGcuvTTOxJCx/JrmQRkuIiWsdjIC6R+22TFI8qs2xzrTxRf/4brVVFYJJLcz2etxORAE5Ab4= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=hartkopp.net; spf=fail smtp.mailfrom=hartkopp.net; dkim=pass (2048-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b=SrVmDKaS; dkim=permerror (0-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b=Y8jVIjMA; arc=pass smtp.client-ip=85.215.255.20 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=hartkopp.net Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=hartkopp.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b="SrVmDKaS"; dkim=permerror (0-bit key) header.d=hartkopp.net header.i=@hartkopp.net header.b="Y8jVIjMA" ARC-Seal: i=1; a=rsa-sha256; t=1783673286; cv=none; d=strato.com; s=strato-dkim-0002; b=dVT+IqDmE4Da0cZu94Vz3kNxboTTF4j0iDZnxTqgCOAKyDI5sz9NQ9UPqP/1cEhTuU /rkIwSF/YwozqCqSIyQRo+sPem4m2NQu2QydKjuEF44ma1Iwela09CF3YwdtO3W6Dq/r b/vt4aUQadEjKnkaWGSgpV8fS7kI5lHt9Cj36Plc5rpeclcngTHxkyNaVInWNwvoRxkM JD6z4e6zY/hyLf/ZnSMYHUueo/H5s8syk91ffDwkpF9f0zUGDNNDJDK/+giV4lMAigmp ERvXqAXTFKooJfQb/NiZkh5ArbjdIcmZc8zUxvZkThl3i5ZzU52gcEK8NXMGdWB1YuWA TCyQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1783673286; s=strato-dkim-0002; d=strato.com; h=Message-ID:Date:Subject:Cc:To:From:Cc:Date:From:Subject:Sender; bh=9qYrAyahVQm+QCACc0bP3MsU8p5taox40VDtVUaWnZw=; b=ohWcN2Pgz04gXXLcfyePsJJVpcH0IV3pjF1TUzVqY8Tco960nxamxCh68uOLhh72Mu Vqw4YSpYGj2KPWqm51BN74CeTFEgxiHWLobSZTMwT1WkyHS4bYjyX2ujQOl1lupRsa3s qSwjFqQCE5+vcXT2TxmErXsWE8aM0S/SWorCqCeHSNSq3icQjwlZbbuSy3/0gQkpvuDk h4aDSb63i79aEhhbxJw9EnA93fILLbTApKixujg6zboaCZ3W7cwJTw15gwVCG+3dGk+O R3TBZx7hbCWlUyzoUnxA9vuo6OVh3aOKVqYxSzPaifu/27Rsv5xkHhth9LP2mowdQ/NM 46wQ== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo00 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1783673286; s=strato-dkim-0002; d=hartkopp.net; h=Message-ID:Date:Subject:Cc:To:From:Cc:Date:From:Subject:Sender; bh=9qYrAyahVQm+QCACc0bP3MsU8p5taox40VDtVUaWnZw=; b=SrVmDKaSHfFRIKIp+HrxSH9LTxyUT6ByIrwzuKo5K1bjKWJr4cqqcvELSswgY+KDFX d8KZNzppPCysynYVsZ7pZq/aEtbojKYodAboeWhf7Z4dFmHiJaraAS4a+bO2GQzvv0mv 2GS6xLyRoJaeIC99U9qpQ/w0KruAkaJ+TYqwVbLAMowo3fi70fgqKzSyF0ePe7z2ClKP 0sPzBvC+BbSu+uaXXDuk9LqPOgY6j6uxu/tdGtyCZ3h0ggedqBDIKVIMfGIYD0A/2PzF 4WDlZrKBV0M/yo0HvDkZtoVGx2a1N4w+sXIiwXCnV9V3cRKidhAY9MWt9ySj0kelVrVT JYyQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1783673286; s=strato-dkim-0003; d=hartkopp.net; h=Message-ID:Date:Subject:Cc:To:From:Cc:Date:From:Subject:Sender; bh=9qYrAyahVQm+QCACc0bP3MsU8p5taox40VDtVUaWnZw=; b=Y8jVIjMApYk/vWjpFWWi70ddgLChXIVwbhPM6Obwn1S/7fFwz39u96Q0ccj7HNrOPe FVbdnUTB3wMbfrtTvdBQ== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjH4JKvMdQv2tRkI16oOSW1Ti/f4PoH8=" Received: from vivo.lan by smtp.strato.de (RZmta 55.5.6 DYNA|AUTH) with ESMTPSA id Ka9fdb26A8m65au (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Fri, 10 Jul 2026 10:48:06 +0200 (CEST) From: Oliver Hartkopp To: linux-can@vger.kernel.org Cc: Oliver Hartkopp , sashiko-bot@kernel.org Subject: [PATCH] can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER Date: Fri, 10 Jul 2026 10:47:51 +0200 Message-ID: <20260710084751.73057-1-socketcan@hartkopp.net> X-Mailer: git-send-email 2.53.0 Precedence: bulk X-Mailing-List: linux-can@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="us-ascii" 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. Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Reported-by: sashiko-bot@kernel.org Link: https://lore.kernel.org/linux-can/20260707101420.47F261F000E9@smtp.kernel.org/ Signed-off-by: Oliver Hartkopp --- net/can/isotp.c | 68 +++++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index d30937345bcd..9db8f9f12a0d 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -150,10 +150,12 @@ struct tpcon { 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; ktime_t lastrxcf_tstamp; struct hrtimer rxtimer, txtimer, txfrtimer; @@ -1217,32 +1219,34 @@ static int isotp_release(struct socket *sock) spin_lock(&isotp_notifier_lock); } 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; - - 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); - } - } + /* 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); + + 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 * for in-flight isotp_rcv() callers to finish, so this call must not * be skipped just because so->bound is already 0 here. @@ -1251,13 +1255,10 @@ static int isotp_release(struct socket *sock) hrtimer_cancel(&so->txfrtimer); hrtimer_cancel(&so->txtimer); hrtimer_cancel(&so->rxtimer); - so->ifindex = 0; - so->bound = 0; - sock_orphan(sk); sock->sk = NULL; release_sock(sk); sock_prot_inuse_add(net, sk->sk_prot, -1); @@ -1308,10 +1309,11 @@ 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) { err = -EINVAL; goto out; @@ -1327,18 +1329,16 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l if (!dev) { err = -ENODEV; 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; ifindex = dev->ifindex; @@ -1352,20 +1352,29 @@ static int isotp_bind(struct socket *sock, struct sockaddr_unsized *uaddr, int l /* register for echo skb's */ 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; if (!sock_flag(sk, SOCK_DEAD)) sk_error_report(sk); @@ -1564,11 +1573,11 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg, struct sock *sk = &so->sk; if (!net_eq(dev_net(dev), sock_net(sk))) return; - if (so->ifindex != dev->ifindex) + if (so->dev != dev) return; switch (msg) { case NETDEV_UNREGISTER: lock_sock(sk); @@ -1580,14 +1589,16 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg, isotp_rcv, sk); 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; if (!sock_flag(sk, SOCK_DEAD)) sk_error_report(sk); @@ -1643,10 +1654,11 @@ static int isotp_init(struct sock *sk) { struct isotp_sock *so = isotp_sk(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; so->opt.rx_ext_address = CAN_ISOTP_DEFAULT_EXT_ADDRESS; so->opt.rxpad_content = CAN_ISOTP_DEFAULT_PAD_CONTENT; -- 2.53.0