[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