[PATCH net] net: pktgen: keep device lookup under RCU protection

Chengfeng Ye <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
pktgen_find_dev() releases its RCU read-side critical section before
returning pkt_dev. __pktgen_NN_threads() then sets removal_mark through
that unprotected pointer.

The worker can concurrently remove the device and queue it for RCU
freeing:

  CPU 0 (netdevice unregister)     CPU 1 (kpktgend)
  rcu_read_lock()
  find pkt_dev
  rcu_read_unlock()
                                   list_del_rcu(&pkt_dev->list)
                                   kfree_rcu(pkt_dev, rcu)
                                   RCU grace period ends
  pkt_dev->removal_mark = 1

The mutex held by CPU 0 does not cover the worker and does not delay an
RCU grace period, so the final write can access freed memory. KASAN
reported:

  BUG: KASAN: slab-use-after-free in __pktgen_NN_threads+0x241/0x280
  Write of size 4 at addr ffff88810dab804c
  Call Trace:
   __pktgen_NN_threads+0x241/0x280
   pktgen_device_event+0x24e/0x3d0
   unregister_netdevice_many_notify+0xde8/0x1ec0
   rtnl_dellink+0x35d/0xa90
  Allocated by task 92:
   __kasan_kmalloc+0x8f/0xa0
   pktgen_thread_write+0x498/0x14e0
  Freed by task 0:
   __kasan_slab_free+0x43/0x70
   rcu_core+0x50a/0x1850

Move the existing RCU read lock into the sole caller and release it only
after setting removal_mark. The object therefore remains alive through
the dereference, while lookup order and control handling remain
unchanged.

Fixes: 8788370a1d4b ("pktgen: RCU-ify "if_list" to remove lock in next_to_run()")
Cc: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
---
 net/core/pktgen.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 7f81aed46672..4fb1853589b3 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2032,14 +2032,17 @@ static struct pktgen_dev *__pktgen_NN_threads(const struct pktgen_net *pn,
 	bool exact = (remove == FIND);
 
 	list_for_each_entry(t, &pn->pktgen_threads, th_list) {
+		rcu_read_lock();
 		pkt_dev = pktgen_find_dev(t, ifname, exact);
 		if (pkt_dev) {
 			if (remove) {
 				pkt_dev->removal_mark = 1;
 				t->control |= T_REMDEV;
 			}
-			break;
 		}
+		rcu_read_unlock();
+		if (pkt_dev)
+			break;
 	}
 	return pkt_dev;
 }
@@ -3776,7 +3779,6 @@ static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
 	struct pktgen_dev *p, *pkt_dev = NULL;
 	size_t len = strlen(ifname);
 
-	rcu_read_lock();
 	list_for_each_entry_rcu(p, &t->if_list, list)
 		if (strncmp(p->odevname, ifname, len) == 0) {
 			if (p->odevname[len]) {
@@ -3787,7 +3789,6 @@ static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
 			break;
 		}
 
-	rcu_read_unlock();
 	pr_debug("find_dev(%s) returning %p\n", ifname, pkt_dev);
 	return pkt_dev;
 }
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.