linux-kernel - Re: [RFC v4 03/18] page_pool: use netmem alloc/put APIs in __page_pool_alloc_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <29f2c375-65e3-4d22-8274-552653222f8d@gmail.com>
Date: Thu, 5 Jun 2025 11:26:59 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Byungchul Park <byungchul@...com>, willy@...radead.org,
 netdev@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 kernel_team@...ynix.com, kuba@...nel.org, almasrymina@...gle.com,
 ilias.apalodimas@...aro.org, harry.yoo@...cle.com, hawk@...nel.org,
 akpm@...ux-foundation.org, davem@...emloft.net, john.fastabend@...il.com,
 andrew+netdev@...n.ch, toke@...hat.com, tariqt@...dia.com,
 edumazet@...gle.com, pabeni@...hat.com, saeedm@...dia.com, leon@...nel.org,
 ast@...nel.org, daniel@...earbox.net, david@...hat.com,
 lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, vbabka@...e.cz,
 rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
 linux-rdma@...r.kernel.org, bpf@...r.kernel.org, vishal.moola@...il.com
Subject: Re: [RFC v4 03/18] page_pool: use netmem alloc/put APIs in
 __page_pool_alloc_page_order()

On 6/4/25 03:52, Byungchul Park wrote:
> Use netmem alloc/put APIs instead of page alloc/put APIs and make it
> return netmem_ref instead of struct page * in
> __page_pool_alloc_page_order().
> 
> Signed-off-by: Byungchul Park <byungchul@...com>
> Reviewed-by: Mina Almasry <almasrymina@...gle.com>
> ---
>   net/core/page_pool.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 4011eb305cee..523354f2db1c 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -518,29 +518,29 @@ static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t g
>   	return false;
>   }
>   
> -static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
> -						 gfp_t gfp)
> +static netmem_ref __page_pool_alloc_page_order(struct page_pool *pool,
> +					       gfp_t gfp)
>   {
> -	struct page *page;
> +	netmem_ref netmem;
>   
>   	gfp |= __GFP_COMP;
> -	page = alloc_pages_node(pool->p.nid, gfp, pool->p.order);
> -	if (unlikely(!page))
> -		return NULL;
> +	netmem = alloc_netmems_node(pool->p.nid, gfp, pool->p.order);
> +	if (unlikely(!netmem))
> +		return 0;
>   
> -	if (pool->dma_map && unlikely(!page_pool_dma_map(pool, page_to_netmem(page), gfp))) {
> -		put_page(page);
> -		return NULL;
> +	if (pool->dma_map && unlikely(!page_pool_dma_map(pool, netmem, gfp))) {
> +		put_netmem(netmem);

It's a bad idea to have {put,get}_netmem in page pool's code, it has a
different semantics from what page pool expects for net_iov. I.e.
instead of releasing the netmem and allowing it to be reallocated by
page pool, put_netmem(niov) will drop a memory provider reference and
leak the net_iov. Depending on implementation it might even underflow
mp refs if a net_iov is ever passed here.

The second problem is that you pass it page_pool_dma_map(), which
works only with struct page and not net_iov, and so it just
unconditionally casts it back to struct page. Which, to be fair,
a pre-existent issue.

This function deals with pages, can we just use pages instead and
cast to netmem when needed? Sth like in this pseudo code:

netmem_ref __page_pool_alloc_page_order() {
	struct page *p = alloc_pages_order();

	netmem = page_to_netmem(p);
	if (!page_pool_dma_map(netmem)) {
		put_page(p);
		return 0;
	}
	return netmem;
}

-- 
Pavel Begunkov