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

[Chengfeng Ye <nicoyip.dev@gmail.com>]

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: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 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