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

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

[Chengfeng Ye]

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

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

[Eric Dumazet]

On Mon, Aug 24, 2026 at 5:23 PM Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
>
> 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>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

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

[Paolo Abeni]

On 8/24/26 5:23 PM, Chengfeng Ye wrote:
> 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;

Side note: returning the RCU protected ptr outside the RCU read lock
safe, as the caller never deference it, but quite confusing.

It would be nice to follow-up on net-next replacing the return type here
with a bool.

/P

[PATCH net-next] net: pktgen: return bool from __pktgen_NN_threads()

[Chengfeng Ye]

Callers of __pktgen_NN_threads() only check whether a pktgen_dev was
found. After the RCU lookup they never dereference the returned
pointer, so returning it past rcu_read_unlock() is confusing.

Return bool instead and keep the pktgen_dev pointer local to the
RCU critical section.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
This is a net-next follow-up to
https://lore.kernel.org/netdev/20260824152331.216494-1-nicoyip.dev@gmail.com/

 net/core/pktgen.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 4fb1853589b3..9985b30c5a42 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2024,11 +2024,11 @@ static const struct proc_ops pktgen_thread_proc_ops = {
 };
 
 /* Think find or remove for NN */
-static struct pktgen_dev *__pktgen_NN_threads(const struct pktgen_net *pn,
-					      const char *ifname, int remove)
+static bool __pktgen_NN_threads(const struct pktgen_net *pn,
+				const char *ifname, int remove)
 {
 	struct pktgen_thread *t;
-	struct pktgen_dev *pkt_dev = NULL;
+	struct pktgen_dev *pkt_dev;
 	bool exact = (remove == FIND);
 
 	list_for_each_entry(t, &pn->pktgen_threads, th_list) {
@@ -2042,9 +2042,9 @@ static struct pktgen_dev *__pktgen_NN_threads(const struct pktgen_net *pn,
 		}
 		rcu_read_unlock();
 		if (pkt_dev)
-			break;
+			return true;
 	}
-	return pkt_dev;
+	return false;
 }
 
 /*
@@ -2052,7 +2052,6 @@ static struct pktgen_dev *__pktgen_NN_threads(const struct pktgen_net *pn,
  */
 static void pktgen_mark_device(const struct pktgen_net *pn, const char *ifname)
 {
-	struct pktgen_dev *pkt_dev = NULL;
 	const int max_tries = 10, msec_per_try = 125;
 	int i = 0;
 
@@ -2061,8 +2060,7 @@ static void pktgen_mark_device(const struct pktgen_net *pn, const char *ifname)
 
 	while (1) {
 
-		pkt_dev = __pktgen_NN_threads(pn, ifname, REMOVE);
-		if (pkt_dev == NULL)
+		if (!__pktgen_NN_threads(pn, ifname, REMOVE))
 			break;	/* success */
 
 		mutex_unlock(&pktgen_thread_lock);
@@ -3836,8 +3834,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 
 	/* We don't allow a device to be on several threads */
 
-	pkt_dev = __pktgen_NN_threads(t->net, ifname, FIND);
-	if (pkt_dev) {
+	if (__pktgen_NN_threads(t->net, ifname, FIND)) {
 		pr_err("ERROR: interface already used\n");
 		return -EBUSY;
 	}
-- 
2.43.0

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

[Chengfeng Ye]

On Tue, Aug 25, 2026 at 6:45 PM Paolo Abeni <pabeni@redhat.com> wrote:
> Side note: returning the RCU protected ptr outside the RCU read lock
> safe, as the caller never deference it, but quite confusing.
>
> It would be nice to follow-up on net-next replacing the return type here
> with a bool.
>
> /P
>

No problem, the follow-up patch is just sent.

Best Regards,
Chengfeng

Re: [PATCH net-next] net: pktgen: return bool from __pktgen_NN_threads()

[Paolo Abeni]

On 8/25/26 8:53 PM, Chengfeng Ye wrote:
> Callers of __pktgen_NN_threads() only check whether a pktgen_dev was
> found. After the RCU lookup they never dereference the returned
> pointer, so returning it past rcu_read_unlock() is confusing.
> 
> Return bool instead and keep the pktgen_dev pointer local to the
> RCU critical section.
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
## Form letter - net-next-closed

net-next pull request for v7.3 has already been merged, and therefore
the net-next tree is closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens after Aug 31st.

RFC patches sent for review only are obviously welcome at any time.

See:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer
pv-bot: closed