lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241009232728.107604-2-edumazet@google.com>
Date: Wed,  9 Oct 2024 23:27:26 +0000
From: Eric Dumazet <edumazet@...gle.com>
To: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>
Cc: Kuniyuki Iwashima <kuniyu@...zon.com>, netdev@...r.kernel.org, eric.dumazet@...il.com, 
	Eric Dumazet <edumazet@...gle.com>
Subject: [PATCH net-next 1/2] netdev-genl: do not use rtnl in netdev_nl_napi_get_doit()

With upcoming per netns RTNL, rtnl use in netdev_nl_napi_get_doit()
is a bit problematic.

Prepare netdev_nl_napi_fill_one() to not rely on RTNL:

1) netif_napi_set_irq() uses WRITE_ONCE(napi->irq, ...)

2) napi_kthread_create() uses WRITE_ONCE(napi->thread, ...)

3) Add napi->thread_pid_nr to avoid race in netdev_nl_napi_fill_one
   and __netif_napi_del()

4) netdev_nl_napi_fill_one() uses corresponding READ_ONCE()

5) netdev_nl_napi_get_doit() can use RCU instead of RTNL

Signed-off-by: Eric Dumazet <edumazet@...gle.com>
---
 include/linux/netdevice.h |  3 ++-
 net/core/dev.c            | 21 ++++++++++++---------
 net/core/netdev-genl.c    | 21 +++++++++++----------
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3baf8e539b6f33caaf83961c4cf619b799e5e41d..64a5e4927901740db8dbc255ed19faca96820333 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -377,6 +377,7 @@ struct napi_struct {
 	struct list_head	dev_list;
 	struct hlist_node	napi_hash_node;
 	int			irq;
+	pid_t			thread_pid_nr;
 };
 
 enum {
@@ -2618,7 +2619,7 @@ void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
 
 static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
 {
-	napi->irq = irq;
+	WRITE_ONCE(napi->irq, irq);
 }
 
 /* Default NAPI poll() weight
diff --git a/net/core/dev.c b/net/core/dev.c
index ea5fbcd133ae4c743545945def00790ec74e2bb6..77c39a95e74df2485777bc008a507bdcc4e75a00 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1423,21 +1423,23 @@ static int napi_threaded_poll(void *data);
 
 static int napi_kthread_create(struct napi_struct *n)
 {
-	int err = 0;
+	struct task_struct *thread;
 
 	/* Create and wake up the kthread once to put it in
 	 * TASK_INTERRUPTIBLE mode to avoid the blocked task
 	 * warning and work with loadavg.
 	 */
-	n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
+	thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
 				n->dev->name, n->napi_id);
-	if (IS_ERR(n->thread)) {
-		err = PTR_ERR(n->thread);
+	if (IS_ERR(thread)) {
+		int err = PTR_ERR(thread);
+
 		pr_err("kthread_run failed with err %d\n", err);
-		n->thread = NULL;
+		return err;
 	}
-
-	return err;
+	WRITE_ONCE(n->thread, thread);
+	WRITE_ONCE(n->thread_pid_nr, task_pid_nr(thread));
+	return 0;
 }
 
 static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack)
@@ -6668,6 +6670,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 	set_bit(NAPI_STATE_SCHED, &napi->state);
 	set_bit(NAPI_STATE_NPSVC, &napi->state);
 	list_add_rcu(&napi->dev_list, &dev->napi_list);
+	netif_napi_set_irq(napi, -1);
 	napi_hash_add(napi);
 	napi_get_frags_check(napi);
 	/* Create kthread for this napi if dev->threaded is set.
@@ -6676,7 +6679,6 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 	 */
 	if (dev->threaded && napi_kthread_create(napi))
 		dev->threaded = false;
-	netif_napi_set_irq(napi, -1);
 }
 EXPORT_SYMBOL(netif_napi_add_weight);
 
@@ -6753,7 +6755,8 @@ void __netif_napi_del(struct napi_struct *napi)
 
 	if (napi->thread) {
 		kthread_stop(napi->thread);
-		napi->thread = NULL;
+		WRITE_ONCE(napi->thread, NULL);
+		WRITE_ONCE(napi->thread_pid_nr, 0);
 	}
 }
 EXPORT_SYMBOL(__netif_napi_del);
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 1cb954f2d39e8248bffd854cdf27eceb25293425..0dcfe3527c122884c5713e56d5e27d4e638d936f 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -163,10 +163,11 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 {
 	void *hdr;
 	pid_t pid;
+	int irq;
 
 	if (WARN_ON_ONCE(!napi->dev))
 		return -EINVAL;
-	if (!(napi->dev->flags & IFF_UP))
+	if (!(READ_ONCE(napi->dev->flags) & IFF_UP))
 		return 0;
 
 	hdr = genlmsg_iput(rsp, info);
@@ -177,17 +178,17 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 	    nla_put_u32(rsp, NETDEV_A_NAPI_ID, napi->napi_id))
 		goto nla_put_failure;
 
-	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
+	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX,
+			READ_ONCE(napi->dev->ifindex)))
 		goto nla_put_failure;
 
-	if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
+	irq = READ_ONCE(napi->irq);
+	if (irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, irq))
 		goto nla_put_failure;
 
-	if (napi->thread) {
-		pid = task_pid_nr(napi->thread);
-		if (nla_put_u32(rsp, NETDEV_A_NAPI_PID, pid))
-			goto nla_put_failure;
-	}
+	pid = READ_ONCE(napi->thread_pid_nr);
+	if (pid && nla_put_u32(rsp, NETDEV_A_NAPI_PID, pid))
+		goto nla_put_failure;
 
 	genlmsg_end(rsp, hdr);
 
@@ -214,7 +215,7 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 	if (!rsp)
 		return -ENOMEM;
 
-	rtnl_lock();
+	rcu_read_lock();
 
 	napi = napi_by_id(napi_id);
 	if (napi) {
@@ -224,7 +225,7 @@ int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 		err = -ENOENT;
 	}
 
-	rtnl_unlock();
+	rcu_read_unlock();
 
 	if (err)
 		goto err_free_msg;
-- 
2.47.0.rc0.187.ge670bccf7e-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ