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

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: [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;

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