From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.remote-ip=81.169.146.168 ARC-Seal: i=2; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=fail; t=1794333705; b=MVz3CaNihVTqrQnLQkjJQ1wHvG72l++v9LuGvFDSMH3spbg0KUJTdLy2BfBxSvNDy4Sr Bf1GM091SNCNPybYjIIqeKnv57VLEaZatDsnBZxSWIwvl8XolnjiZQzZOidd3ikDzevBH xd3qPGpfAYDqx/JV4om9OBv3XlI1y4snm5DLDB0tKvlvijtnO5CufWIlopgMDtmKnhKht GTGBQodl0jHebydClnEOGuPwV2dCFhheTh8bQQhC5ATMyFFQbzoXZ6NKHCrQsy7ofMgVt xzRNMouegDlGbzeE//6NC0bGn0W2A7uEaT4CRBWBhJwmBBuByUfW4hBzNF4rDpHmSrA== ARC-Message-Signature: i=2; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1794333705; h=DMARC-Filter:X-RZG-CLASS-ID:DKIM-Signature:DKIM-Signature:X-RZG-AUTH: Received:From:To:Cc:Subject:Date:Message-ID:X-Mailer:In-Reply-To: References:MIME-Version:Content-Transfer-Encoding:Content-Type; bh=LtkD/xb+D48baBHkjZ/mtYVdHNvOyjOZDv6CEFJSNRo=; b=MTDuCa9Qz1qsFZ2Cqd5FtRB9ELarMJQ1+p39rmAjUOgEi64wuLHiGNLmQtsnCTVvIYsO WrSzGjsNxtUr1h1jLJ4OpY4VHTuHG+X+kJby14E4bHHTEc1NHBpL10q74pMBCV1pOR91d Y5EjORaYAPIuhUFdEpzjhzH+zxDqDnTutLYDaQNNlbSMMSjSd+mCrm5MhCAb0XYm5dNuR qpIxwqyaojC2yu2VTPAS7Zw8gV87nLUrJH6bmAeF3JbIfmRh1NQRLeSKqxElcyscSUU2H JqZptKRMFjKtZzy48e5Q0p2MWJESV19jXhzgFmBxyNrdLtBYp6XWOHi6oXJ+8UKZhJQ== ARC-Authentication-Results: i=2; smtp.subspace.kernel.org; dmarc=pass header.from=hartkopp.net; spf=fail smtp.mailfrom=hartkopp.net; dkim=pass header.d=hartkopp.net header.i=@hartkopp.net header.a=rsa-sha256 header.s=strato-dkim-0002 header.b=TAhJrGem; dkim=pass header.d=hartkopp.net header.i=@hartkopp.net header.a=ed25519-sha256 header.s=strato-dkim-0003 header.b=3q6J7zXM; arc=fail smtp.remote-ip=81.169.146.168 Received: from mo4-p02-ob.smtp.rzone.de (mo4-p02-ob.smtp.rzone.de [81.169.146.168]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A12A4606A326 for ; Tue, 10 Nov 2026 18:01:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org A12A4606A326 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.a=rsa-sha256 header.s=strato-dkim-0002 header.b=TAhJrGem; dkim=pass header.d=hartkopp.net header.i=@hartkopp.net header.a=ed25519-sha256 header.s=strato-dkim-0003 header.b=3q6J7zXM ARC-Seal: i=1; a=rsa-sha256; t=1794333681; cv=none; d=strato.com; s=strato-dkim-0002; b=m+4imKrPgqZ0EZ5A2UjAtlAxcSHdeGpQ2+etx8bKu/IEbI9pfTtaQP3ntSA7hLo/iX 8j09F7iT1HzHIyLNLbaxx6Tg9Klvrp+oJDiUadf3XhSXa9tw3YAfMn82uN8eysBWZYcx eSkRJdxAXrMVQHeKPHHuiGbWazHxYtzeZcKQucie44dNepoU73Svl8G7P+ih/sjsM7OZ fUBioVuGiT7Ozfo1Q6dKT1QiaFT37jjl4fFYEcj8H3C61t4ux19/3AxObEnf7wmLedwN maoadlNx7W9VRP7J+l45wFD76tLw2UVwfBmVPMVlvUmtVY3GBb1/k8m6idvYqG8SVWuI TscA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1794333681; s=strato-dkim-0002; d=strato.com; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=LtkD/xb+D48baBHkjZ/mtYVdHNvOyjOZDv6CEFJSNRo=; b=LjPcJWV3cl9YI+G0dAsQV70dgEb77Np+3EA1uixRKt9EpT15TFGSFxsx/1mijBceqL 4mXyRFRro+A1saK60IeIYhwJQdz3BeXZpk+mivRNjiYYk5wKQ2coQJ4oyy34R4qATjrb XUfve2jLkLqX9jihTvFp8wB+RWxZR/fmc3dF8JHV4jYj50rNuC43e0KBafWxzisi+vlc y+SV/Y0+PGmLBTl+8PfSwxBGPz+dvaxeEHMUAreERNbBCG/GdNen+37DTqvi3VGAq5iW UzAdpt2JwiBfRcHdEn0QzsVtSgRiriW52Yq+aCx4cPFIUS7UWBwclZnG7q4pYoVGCSpa mXVg== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo02 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1794333681; s=strato-dkim-0002; d=hartkopp.net; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=LtkD/xb+D48baBHkjZ/mtYVdHNvOyjOZDv6CEFJSNRo=; b=TAhJrGemDozWqoqGJEkUjdMjr23vqcXig69hrdBSawzNpVNq8r15mLEXhE4+AQrsoG 4oBRKlCLjDbuH98MtII3w2Ihh7g5Q8zcULjB0w4rlLzYDE3r+oTaDJXj3/6DZ2QSWLX4 cF2ks9NoM1/XQaBxPnOdkRi/q1QEqZXlerNyZNcHqFPPFciPDDyjiUJTyAvw9JmOoQWh DsFLTHPGKdPd5RTn6KI3i/tuhOnMzqCYnXAiy8qQNTsFKMKXxl+OW9CorTcBJHVgzs4Z xvs9PxdlQawkaPkLGrat/hKxQ/iM1XFg/zFYbiD03hc3sauJ5+60FOCKEdrGsWlcBFeM YZfg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1794333681; s=strato-dkim-0003; d=hartkopp.net; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date: From:Subject:Sender; bh=LtkD/xb+D48baBHkjZ/mtYVdHNvOyjOZDv6CEFJSNRo=; b=3q6J7zXMz4XiZA/aX3K71HtcBUMeE1AEW+eCFzg9eADbbDKvjyOhyglSkk5GqIFkS+ EEP6UDiqUFYfOfppCSCw== X-RZG-AUTH: ":P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrDxb8mjH4JKvMdQv2tTUsMrZpkO3Mw3lZ/t54cFxeEQ7s8bDup0Q==" Received: from vivo.lan by smtp.strato.de (RZmta 55.6.3 AUTH) with ESMTPSA id Kfdf2a2AAI1L1ct (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Tue, 10 Nov 2026 19:01:21 +0100 (CET) From: Oliver Hartkopp To: stable@vger.kernel.org Cc: Sasha Levin , Marc Kleine-Budde , Oliver Hartkopp Subject: [PATCH 6.6.y] can: protect the receive lists with a per-device lock Date: Tue, 10 Nov 2026 19:01:14 +0100 Message-ID: <20261110180114.792016-1-socketcan@hartkopp.net> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20261109201321-failed-6.6-0f3ad9ab40e3@kernel.org> References: <20261109201321-failed-6.6-0f3ad9ab40e3@kernel.org> X-Mailing-List: stable@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="us-ascii" commit 0f3ad9ab40e3dad06d5c0693806e07935ceedbc3 upstream. The receive lists of a CAN device are located in the device's can_ml_priv and travel with the device when it is moved to another network namespace. But they are protected by the per-netns rcvlists_lock. So can_rx_unregister() needs dev_net(dev) to pick the right lock and silently returns when the passed namespace does not match dev_net(dev) anymore. isotp_release() and bcm_release() leave their notifier list before they take any lock that serializes against the netdev notifier, which runs under RTNL. When the bound device is moved to another namespace in this window, the notifier skips the socket and the release path has to remove the filters itself. isotp_release() passes the socket's namespace, which does not match dev_net(dev) anymore. bcm_rx_unreg() passes dev_net(dev), which can_rx_unregister() reads again for its check, so a namespace change between both reads makes the check fail as well. In both cases can_rx_unregister() silently returns. As can_rx_register() stores rcv->sk without a reference, the remaining receivers point to the freed socket (isotp) or the freed op (bcm): BUG: KASAN: use-after-free in isotp_rcv+0x1570/0x24d0 Read of size 1 at addr ffff888118130552 by task isotp_ns_uaf/578 can_rcv_filter+0x4af/0x8c0 can_receive+0x28d/0x3c0 can_rcv+0x2a9/0x310 __netif_receive_skb_one_core+0x21a/0x260 process_backlog+0x210/0x760 Besides this, the lists of a device are protected by the rcvlists_lock of the namespace that is passed in. Callers which pass dev_net(dev) read it at different times, so the same lists may be modified under the locks of different namespaces while the device is moved. Taking RTNL in these release paths is no option: raw_bind(), raw_setsockopt() and raw_release() already hold RTNL while adding and removing up to CAN_RAW_FILTER_MAX filters per socket, which unprivileged users can trigger repeatedly and which makes RTNL a global bottleneck. Instead fix the root cause and add a spinlock to struct can_dev_rcv_lists, which protects the receive lists of a CAN device or the 'any' CAN device lists of a namespace. It is initialized in can_set_ml_priv(), which is used by all CAN network devices. With this, can_rx_unregister() finds the subscription independently of the current namespace of the device. The protocols keep the device valid with their tracked references. The per-netns rcvlists_lock only protects the receive list statistics anymore, and CAN_RAW, CAN_BCM and CAN_ISOTP pass the namespace that has been used for can_rx_register(). So isotp_release() and bcm_release() now remove their filters correctly without RTNL, and raw_release(), raw_setsockopt(), raw_bind() and isotp_bind() do not need RTNL anymore and add and remove their filters under lock_sock(). To serialize bind() with NETDEV_UNREGISTER, bind() publishes the device inside the RCU section of its lookup, and raw_notify() and isotp_notify() recheck the bound device under lock_sock(). As RTNL does not keep NETDEV_DOWN out of bind() anymore, bind() checks IFF_UP after publishing the bound device, with a barrier pair against raw_notify() and isotp_notify(), so a down transition during bind() is still reported with ENETDOWN. Fixes: 20bab8b88baa ("can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER") Fixes: 8e8cda6d737d ("can: initial support for network namespaces") Reported-by: Norbert Szetei Link: https://lore.kernel.org/linux-can/CEA6A38A-2646-4ADA-95B4-CBAE2F301A8E@doyensec.com/ Cc: stable@vger.kernel.org Signed-off-by: Oliver Hartkopp Link: https://patch.msgid.link/20261027-can-rx-lock-v12-3-eb5afc2d8b9b@hartkopp.net Signed-off-by: Marc Kleine-Budde [ backport to 6.6: Adapt the can_pernet_init() context to kzalloc() as this tree has no kzalloc_obj(). so->dev holds an untracked reference with dev_hold() in this tree. raw_setsockopt() has no raw_setsockopt_locked() split from commit 1e5185c09058 ("can: raw: add locking for raw flags bitfield") in this tree, so remove the RTNL locking from the CAN_RAW_FILTER and CAN_RAW_ERR_FILTER cases directly. Keep the NULL check for dev_put() in raw_bind() and use sock_net(sk) in raw_release(). ] Signed-off-by: Oliver Hartkopp --- This backport depends on the backport of 962947b69589 ("can: replace ARPHRD_CAN type checks with can_get_ml_priv()") sent in reply to <20261109201316-failed-6.6-962947b69589@kernel.org> include/linux/can/can-ml.h | 3 ++ include/net/netns/can.h | 2 +- net/can/af_can.c | 39 +++++++++----- net/can/bcm.c | 43 +++++---------- net/can/isotp.c | 89 +++++++++++++++++++------------ net/can/raw.c | 106 +++++++++++++++++++++++++------------ 6 files changed, 166 insertions(+), 116 deletions(-) diff --git a/include/linux/can/can-ml.h b/include/linux/can/can-ml.h index 8afa92d15a66..f10444c6115f 100644 --- a/include/linux/can/can-ml.h +++ b/include/linux/can/can-ml.h @@ -45,6 +45,7 @@ #include #include #include +#include #define CAN_SFF_RCV_ARRAY_SZ (1 << CAN_SFF_ID_BITS) #define CAN_EFF_RCV_HASH_BITS 10 @@ -53,6 +54,7 @@ enum { RX_ERR, RX_ALL, RX_FIL, RX_INV, RX_MAX }; struct can_dev_rcv_lists { + spinlock_t lock; /* protects the receive lists and entries */ struct hlist_head rx[RX_MAX]; struct hlist_head rx_sff[CAN_SFF_RCV_ARRAY_SZ]; struct hlist_head rx_eff[CAN_EFF_RCV_ARRAY_SZ]; @@ -74,6 +76,7 @@ static inline struct can_ml_priv *can_get_ml_priv(struct net_device *dev) static inline void can_set_ml_priv(struct net_device *dev, struct can_ml_priv *ml_priv) { + spin_lock_init(&ml_priv->dev_rcv_lists.lock); netdev_set_ml_priv(dev, ml_priv, ML_PRIV_CAN); } diff --git a/include/net/netns/can.h b/include/net/netns/can.h index 48b79f7e6236..54f23681e0d4 100644 --- a/include/net/netns/can.h +++ b/include/net/netns/can.h @@ -29,7 +29,7 @@ struct netns_can { /* receive filters subscribed for 'all' CAN devices */ struct can_dev_rcv_lists *rx_alldev_list; - spinlock_t rcvlists_lock; + spinlock_t rcvlists_lock; /* protects the rcv_entries(_max) stats */ struct timer_list stattimer; /* timer for statistics update */ struct can_pkg_stats *pkg_stats; struct can_rcv_lists_stats *rcv_lists_stats; diff --git a/net/can/af_can.c b/net/can/af_can.c index d96f76bea4e2..7428a92aef45 100644 --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -460,9 +460,10 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, if (!rcv) return -ENOMEM; - spin_lock_bh(&net->can.rcvlists_lock); - dev_rcv_lists = can_dev_rcv_lists_find(net, dev); + + spin_lock_bh(&dev_rcv_lists->lock); + rcv_list = can_rcv_list_find(&can_id, &mask, dev_rcv_lists); rcv->can_id = can_id; @@ -476,6 +477,9 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id, hlist_add_head_rcu(&rcv->list, rcv_list); dev_rcv_lists->entries++; + spin_unlock_bh(&dev_rcv_lists->lock); + + spin_lock_bh(&net->can.rcvlists_lock); rcv_lists_stats->rcv_entries++; rcv_lists_stats->rcv_entries_max = max(rcv_lists_stats->rcv_entries_max, rcv_lists_stats->rcv_entries); @@ -507,6 +511,14 @@ static void can_rx_delete_receiver(struct rcu_head *rp) * * Description: * Removes subscription entry depending on given (subscription) values. + * The receive lists of a CAN device are protected by a per-device lock, + * so the subscription is found even when the device has been moved to + * another network namespace after can_rx_register(). The caller has to + * ensure that @dev stays valid, e.g. by holding a reference to @dev or + * RTNL. @net should be the namespace that has been used for + * can_rx_register(). It selects the 'any' CAN device lists for + * @dev == NULL and is otherwise only used for the receive list + * statistics, which become imprecise when another namespace is passed. */ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id, canid_t mask, void (*func)(struct sk_buff *, void *), @@ -520,12 +532,10 @@ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id, if (dev && !can_get_ml_priv(dev)) return; - if (dev && !net_eq(net, dev_net(dev))) - return; + dev_rcv_lists = can_dev_rcv_lists_find(net, dev); - spin_lock_bh(&net->can.rcvlists_lock); + spin_lock_bh(&dev_rcv_lists->lock); - dev_rcv_lists = can_dev_rcv_lists_find(net, dev); rcv_list = can_rcv_list_find(&can_id, &mask, dev_rcv_lists); /* Search the receiver list for the item to delete. This should @@ -545,26 +555,26 @@ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id, * a warning here. */ if (!rcv) { + spin_unlock_bh(&dev_rcv_lists->lock); pr_warn("can: receive list entry not found for dev %s, id %03X, mask %03X\n", DNAME(dev), can_id, mask); - goto out; + return; } hlist_del_rcu(&rcv->list); dev_rcv_lists->entries--; + spin_unlock_bh(&dev_rcv_lists->lock); + + spin_lock_bh(&net->can.rcvlists_lock); if (rcv_lists_stats->rcv_entries > 0) rcv_lists_stats->rcv_entries--; - - out: spin_unlock_bh(&net->can.rcvlists_lock); /* schedule the receiver item for deletion */ - if (rcv) { - if (rcv->sk) - sock_hold(rcv->sk); - call_rcu(&rcv->rcu, can_rx_delete_receiver); - } + if (rcv->sk) + sock_hold(rcv->sk); + call_rcu(&rcv->rcu, can_rx_delete_receiver); } EXPORT_SYMBOL(can_rx_unregister); @@ -795,6 +805,7 @@ static int can_pernet_init(struct net *net) kzalloc(sizeof(*net->can.rx_alldev_list), GFP_KERNEL); if (!net->can.rx_alldev_list) goto out; + spin_lock_init(&net->can.rx_alldev_list->lock); net->can.pkg_stats = kzalloc(sizeof(*net->can.pkg_stats), GFP_KERNEL); if (!net->can.pkg_stats) goto out_free_rx_alldev_list; diff --git a/net/can/bcm.c b/net/can/bcm.c index bfdaab08c423..1bc7c234c1ab 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -915,7 +915,7 @@ static void bcm_remove_op(struct bcm_op *op) static void bcm_rx_unreg(struct net_device *dev, struct bcm_op *op) { if (op->rx_reg_dev == dev) { - can_rx_unregister(dev_net(dev), dev, op->can_id, + can_rx_unregister(sock_net(op->sk), dev, op->can_id, REGMASK(op->can_id), bcm_rx_handler, op); /* mark as removed subscription */ @@ -941,21 +941,10 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh, /* disable automatic timer on frame reception */ op->flags |= RX_NO_AUTOTIMER; - /* - * Don't care if we're bound or not (due to netdev - * problems) can_rx_unregister() is always a save - * thing to do here. - */ if (op->ifindex) { - /* - * 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. + /* Only remove subscriptions that had not been + * removed in bcm_notify(). op->rx_reg_dev keeps + * the device's receive lists accessible. */ if (op->rx_reg_dev) bcm_rx_unreg(op->rx_reg_dev, op); @@ -1885,6 +1874,7 @@ static int bcm_release(struct socket *sock) list_del(&bo->notifier); spin_unlock(&bcm_notifier_lock); + /* bcm_notify() can not access this socket anymore */ lock_sock(sk); #if IS_ENABLED(CONFIG_PROC_FS) @@ -1898,29 +1888,20 @@ static int bcm_release(struct socket *sock) bcm_remove_op(op); } - list_for_each_entry_safe(op, next, &bo->rx_ops, list) { - /* - * Don't care if we're bound or not (due to netdev problems) - * can_rx_unregister() is always a save thing to do here. - */ + list_for_each_entry(op, &bo->rx_ops, list) { if (op->ifindex) { - /* - * 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. + /* Only remove subscriptions that had not been removed + * in bcm_notify(). op->rx_reg_dev keeps the device's + * receive lists accessible. */ if (op->rx_reg_dev) bcm_rx_unreg(op->rx_reg_dev, op); - } else + } else { can_rx_unregister(net, NULL, op->can_id, REGMASK(op->can_id), bcm_rx_handler, op); - + } + cond_resched(); } synchronize_rcu(); diff --git a/net/can/isotp.c b/net/can/isotp.c index 71ba2b7c9a5c..69366f957507 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1111,7 +1111,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) int off; int err; - if (!so->bound || READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN) + if (!READ_ONCE(so->bound) || READ_ONCE(so->tx.state) == ISOTP_SHUTDOWN) return -EADDRNOTAVAIL; /* claim the socket under so->rx_lock: this serializes the claim @@ -1156,7 +1156,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) /* so->bound is only checked once above - a wakeup may have * unbound/rebound the socket meanwhile => recheck */ - if (!so->bound) { + if (!READ_ONCE(so->bound)) { err = -EADDRNOTAVAIL; goto err_out_drop; } @@ -1396,7 +1396,7 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, if (flags & ~(MSG_DONTWAIT | MSG_TRUNC | MSG_PEEK | MSG_CMSG_COMPAT)) return -EINVAL; - if (!so->bound) + if (!READ_ONCE(so->bound)) return -EADDRNOTAVAIL; skb = skb_recv_datagram(sk, flags, &ret); @@ -1470,11 +1470,11 @@ 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 - * tracked reference so->dev is taken at bind() time with rtnl_lock + * the tracked reference so->dev keeps the device's receive lists + * accessible, even after a namespace change */ if (so->bound && so->dev) { if (isotp_register_rxid(so)) @@ -1489,11 +1489,9 @@ static int isotp_release(struct socket *sock) } so->ifindex = 0; - so->bound = 0; + WRITE_ONCE(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 @@ -1559,7 +1557,7 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (!addr->can_ifindex) return -ENODEV; - rtnl_lock(); + /* lock_sock() serializes the binding with isotp_notify() */ lock_sock(sk); if (so->bound) { @@ -1585,19 +1583,24 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) goto out; } - dev = dev_get_by_index(net, addr->can_ifindex); - if (!dev) { - err = -ENODEV; - goto out; - } - if (!can_get_ml_priv(dev)) { + /* Publish the device inside the RCU section of the lookup: When it + * is unlisted concurrently, either isotp_notify() sees so->dev after + * synchronize_net() or the lookup fails. Check the device before + * publishing it. + */ + rcu_read_lock(); + dev = dev_get_by_index_rcu(net, addr->can_ifindex); + if (!dev || !can_get_ml_priv(dev)) err = -ENODEV; - goto out_put_dev; - } - if (READ_ONCE(dev->mtu) < so->ll.mtu) { + else if (READ_ONCE(dev->mtu) < so->ll.mtu) err = -EINVAL; - goto out_put_dev; + if (!err) { + dev_hold(dev); + WRITE_ONCE(so->dev, dev); } + rcu_read_unlock(); + if (err) + goto out; /* switch to new settings */ so->ifindex = dev->ifindex; @@ -1624,35 +1627,39 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) } } - /* report a down device only for a successful bind() */ - if (!(dev->flags & IFF_UP)) - notify_enetdown = 1; - - so->bound = 1; + WRITE_ONCE(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; dev_hold(so->dev); + /* Either isotp_notify() sees so->dev on NETDEV_DOWN or we see the + * cleared IFF_UP, pairs with isotp_notify() + */ + smp_mb(); + if (!(READ_ONCE(dev->flags) & IFF_UP)) + notify_enetdown = 1; + out_put_dev: - if (err) - so->ifindex = 0; /* reset on failed bind() */ + /* revert so->ifindex and so->dev on failed bind() */ + if (err) { + so->ifindex = 0; + WRITE_ONCE(so->dev, NULL); + } - /* remove potential reference from dev_get_by_index() */ + /* remove reference from the device lookup */ 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); } + release_sock(sk); + return err; } @@ -1848,12 +1855,21 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg, if (!net_eq(dev_net(dev), sock_net(sk))) return; - if (so->dev != dev) + if (msg == NETDEV_DOWN) + smp_mb(); /* pairs with isotp_bind(), see raw_notify() */ + + /* lockless fast path, see raw_notify() */ + if (READ_ONCE(so->dev) != dev) return; switch (msg) { case NETDEV_UNREGISTER: lock_sock(sk); + if (so->dev != dev) { + release_sock(sk); + break; + } + /* remove current filters & unregister */ if (so->bound) { if (isotp_register_rxid(so)) @@ -1868,16 +1884,19 @@ static void isotp_notify(struct isotp_sock *so, unsigned long msg, } so->ifindex = 0; - so->bound = 0; - so->dev = NULL; - release_sock(sk); + WRITE_ONCE(so->bound, 0); + WRITE_ONCE(so->dev, NULL); sk->sk_err = ENODEV; if (!sock_flag(sk, SOCK_DEAD)) sk_error_report(sk); + release_sock(sk); break; case NETDEV_DOWN: + if (!READ_ONCE(so->bound)) + break; + sk->sk_err = ENETDOWN; if (!sock_flag(sk, SOCK_DEAD)) sk_error_report(sk); diff --git a/net/can/raw.c b/net/can/raw.c index dd3f4f145beb..ead9e2713a27 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -87,6 +87,7 @@ struct raw_sock { int ifindex; struct net_device *dev; netdevice_tracker dev_tracker; + struct net_device *bind_dev; /* hint for raw_notify(), no reference */ struct list_head notifier; int loopback; int recv_own_msgs; @@ -195,6 +196,7 @@ static int raw_enable_filters(struct net *net, struct net_device *dev, int i; for (i = 0; i < count; i++) { + cond_resched(); err = can_rx_register(net, dev, filter[i].can_id, filter[i].can_mask, raw_rcv, sk, "raw", sk); @@ -229,9 +231,11 @@ static void raw_disable_filters(struct net *net, struct net_device *dev, { int i; - for (i = 0; i < count; i++) + for (i = 0; i < count; i++) { can_rx_unregister(net, dev, filter[i].can_id, filter[i].can_mask, raw_rcv, sk); + cond_resched(); + } } static inline void raw_disable_errfilter(struct net *net, @@ -276,16 +280,31 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg, struct net_device *dev) { struct sock *sk = &ro->sk; + struct net_device *bind_dev; if (!net_eq(dev_net(dev), sock_net(sk))) return; - if (ro->dev != dev) + if (msg == NETDEV_DOWN) + smp_mb(); /* IFF_UP is cleared before, pairs with raw_bind() */ + + /* Lockless fast path, ro->bind_dev covers a concurrent bind(). + * Read it before ro->dev, pairs with smp_store_release() in + * raw_bind(). A stale ro->bind_dev only costs a lock_sock(). + */ + bind_dev = smp_load_acquire(&ro->bind_dev); + if (READ_ONCE(ro->dev) != dev && + (msg != NETDEV_UNREGISTER || bind_dev != dev)) return; switch (msg) { case NETDEV_UNREGISTER: lock_sock(sk); + if (ro->dev != dev) { + release_sock(sk); + break; + } + /* remove current filters & unregister */ if (ro->bound) { raw_disable_allfilters(dev_net(dev), dev, sk); @@ -297,13 +316,13 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg, ro->ifindex = 0; ro->bound = 0; - ro->dev = NULL; + WRITE_ONCE(ro->dev, NULL); ro->count = 0; - release_sock(sk); sk->sk_err = ENODEV; if (!sock_flag(sk, SOCK_DEAD)) sk_error_report(sk); + release_sock(sk); break; case NETDEV_DOWN: @@ -352,6 +371,7 @@ static int raw_init(struct sock *sk) ro->bound = 0; ro->ifindex = 0; ro->dev = NULL; + ro->bind_dev = NULL; /* set default filter to single entry dfilter */ ro->dfilter.can_id = 0; @@ -400,13 +420,15 @@ static int raw_release(struct socket *sock) list_del(&ro->notifier); spin_unlock(&raw_notifier_lock); - rtnl_lock(); lock_sock(sk); - /* remove current filters & unregister */ + /* remove current filters & unregister + * the tracked reference ro->dev keeps the device's receive lists + * accessible, even after a namespace change + */ if (ro->bound) { if (ro->dev) { - raw_disable_allfilters(dev_net(ro->dev), ro->dev, sk); + raw_disable_allfilters(sock_net(sk), ro->dev, sk); netdev_put(ro->dev, &ro->dev_tracker); } else { raw_disable_allfilters(sock_net(sk), NULL, sk); @@ -425,7 +447,6 @@ static int raw_release(struct socket *sock) sock->sk = NULL; release_sock(sk); - rtnl_unlock(); sock_put(sk); @@ -447,14 +468,25 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (addr->can_family != AF_CAN) return -EINVAL; - rtnl_lock(); + /* lock_sock() serializes the binding with raw_notify() */ lock_sock(sk); if (ro->bound && addr->can_ifindex == ro->ifindex) goto out; if (addr->can_ifindex) { - dev = dev_get_by_index(sock_net(sk), addr->can_ifindex); + /* Publish the device inside the RCU section of the lookup: + * When it is unlisted concurrently, either raw_notify() sees + * ro->bind_dev after synchronize_net() or the lookup fails. + */ + rcu_read_lock(); + dev = dev_get_by_index_rcu(sock_net(sk), addr->can_ifindex); + if (dev) { + dev_hold(dev); + /* pairs with smp_load_acquire() in raw_notify() */ + smp_store_release(&ro->bind_dev, dev); + } + rcu_read_unlock(); if (!dev) { err = -ENODEV; goto out; @@ -464,9 +496,6 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len) goto out_put_dev; } - if (!(dev->flags & IFF_UP)) - notify_enetdown = 1; - ifindex = dev->ifindex; /* filters set by default/setsockopt */ @@ -485,7 +514,7 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (ro->bound) { /* unregister old filters */ if (ro->dev) { - raw_disable_allfilters(dev_net(ro->dev), + raw_disable_allfilters(sock_net(sk), ro->dev, sk); /* drop reference to old ro->dev */ netdev_put(ro->dev, &ro->dev_tracker); @@ -496,25 +525,33 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len) ro->ifindex = ifindex; ro->bound = 1; /* bind() ok -> hold a reference for new ro->dev */ - ro->dev = dev; + WRITE_ONCE(ro->dev, dev); if (ro->dev) netdev_hold(ro->dev, &ro->dev_tracker, GFP_KERNEL); + + if (dev) { + /* Either raw_notify() sees ro->dev on NETDEV_DOWN or + * we see the cleared IFF_UP, pairs with raw_notify() + */ + smp_mb(); + if (!(READ_ONCE(dev->flags) & IFF_UP)) + notify_enetdown = 1; + } } out_put_dev: - /* remove potential reference from dev_get_by_index() */ + /* remove reference from the device lookup */ if (dev) 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); } + release_sock(sk); + return err; } @@ -571,17 +608,19 @@ static int raw_setsockopt(struct socket *sock, int level, int optname, return -EFAULT; } - rtnl_lock(); lock_sock(sk); + /* The reg_state check is only a shortcut: On a concurrent + * unregistration or namespace change raw_notify() waits for + * lock_sock() and removes the filters set here. + */ dev = ro->dev; - if (ro->bound && dev) { - if (dev->reg_state != NETREG_REGISTERED) { - if (count > 1) - kfree(filter); - err = -ENODEV; - goto out_fil; - } + if (ro->bound && dev && + READ_ONCE(dev->reg_state) != NETREG_REGISTERED) { + if (count > 1) + kfree(filter); + err = -ENODEV; + goto out_fil; } if (ro->bound) { @@ -618,7 +657,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname, out_fil: release_sock(sk); - rtnl_unlock(); break; @@ -631,15 +669,14 @@ static int raw_setsockopt(struct socket *sock, int level, int optname, err_mask &= CAN_ERR_MASK; - rtnl_lock(); lock_sock(sk); + /* lockless shortcut, see CAN_RAW_FILTER */ dev = ro->dev; - if (ro->bound && dev) { - if (dev->reg_state != NETREG_REGISTERED) { - err = -ENODEV; - goto out_err; - } + if (ro->bound && dev && + READ_ONCE(dev->reg_state) != NETREG_REGISTERED) { + err = -ENODEV; + goto out_err; } /* remove current error mask */ @@ -661,7 +698,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname, out_err: release_sock(sk); - rtnl_unlock(); break;