Re: [PATCH net] page_pool: Fix use-after-free in page_pool_recycle_in_ring

[dongchenchen (A)]

在 2025/5/23 21:31, Paolo Abeni 写道:
> On 5/23/25 8:45 AM, Dong Chenchen wrote:
> > diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > index 7745ad924ae2..08f1b000ebc1 100644
> > --- a/net/core/page_pool.c
> > +++ b/net/core/page_pool.c
> > @@ -707,19 +707,16 @@ void page_pool_return_page(struct page_pool *pool, netmem_ref netmem)
> >   
> >   static bool page_pool_recycle_in_ring(struct page_pool *pool, netmem_ref netmem)
> >   {
> > +	bool in_softirq;
> >   	int ret;
> >   	/* BH protection not needed if current is softirq */
> > -	if (in_softirq())
> > -		ret = ptr_ring_produce(&pool->ring, (__force void *)netmem);
> > -	else
> > -		ret = ptr_ring_produce_bh(&pool->ring, (__force void *)netmem);
> > -
> > -	if (!ret) {
> > +	in_softirq = page_pool_producer_lock(pool);
> > +	ret = !__ptr_ring_produce(&pool->ring, (__force void *)netmem);
> > +	if (ret)
> >   		recycle_stat_inc(pool, ring);
> > -		return true;
> > -	}
> Does not build in our CI:
>
> net/core/page_pool.c: In function ‘page_pool_recycle_in_ring’:
> net/core/page_pool.c:750:45: error: suggest braces around empty body in
> an ‘if’ statement [-Werror=empty-body]
>    750 |                 recycle_stat_inc(pool, ring);
>        |                                             ^
>
> /P
I am sorry for this mistake.
recycle_stat_inc() is empty when CONFIG_PAGE_POOL_STATS is not enabled.
Maybe we can fix it as:

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 08f1b000ebc1..19c1505ec40f 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -154,8 +154,8 @@ EXPORT_SYMBOL(page_pool_ethtool_stats_get);
 
 #else
 #define alloc_stat_inc(pool, __stat)
-#define recycle_stat_inc(pool, __stat)
-#define recycle_stat_add(pool, __stat, val)
+#define recycle_stat_inc(pool, __stat)         do { } while (0)
+#define recycle_stat_add(pool, __stat, val)    do { } while (0)
 #endif

Thanks a lot!

Dong Chenchen