[PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Jijie Shao]

page_pool_scrub() iterates pool->dma_mapped via xa_for_each() with no
page ref held. __page_pool_release_netmem_dma() currently reads and
writes netmem fields (dma_addr, DMA index bits in pp_magic) after
xa_cmpxchg() returns. The unref path calls put_page() unconditionally
regardless of the cmpxchg outcome; when it loses the cmpxchg, it still
frees the page before the scrub winner finishes these netmem accesses,
so scrub touches a freed page -- a Use-After-Free.

Fix this by splitting the DMA release into two functions:

1. __page_pool_unmap_netmem_dma() caches dma_addr before xa_cmpxchg(),
   does the cmpxchg to remove the DMA mapping, and calls dma_unmap on
   the cached address. It never touches netmem fields after the cmpxchg,
   making it safe for the scrub path which holds no page ref.

2. __page_pool_release_netmem_dma() wraps the above and additionally
   clears dma_addr and DMA index bits in netmem fields. This is safe
   only when the caller holds a page ref, so it is used by the return
   path (page_pool_return_netmem).

The scrub path calls __page_pool_unmap_netmem_dma() directly; the return
path calls __page_pool_release_netmem_dma().

Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
Suggested-by: Mina Almasry <almasrymina@google.com>
Assisted-by: OhMyOpenCode:GLM-5.2
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
Changes in v4:
- Restructure per Mina's review: merge page_pool_remove_dma_mapping()
  into __page_pool_unmap_netmem_dma() with dma_unmap inlined via goto
  label; simplify __page_pool_release_netmem_dma() to a thin wrapper.
- Link to v3: https://lore.kernel.org/r/20260729110249.2824835-1-shaojijie@huawei.com

Changes in v3:
- Fix unlikely() to likely() for PP_DMA_INDEX_BITS to match
  file convention.
- Link to v2: https://lore.kernel.org/r/20260727132612.3277927-1-shaojijie@huawei.com

Changes in v2:
- Redesign the fix per Mina's review: v1's unconditional
  netmem_set_dma_index() introduced a UAF when the scrub path
  (no page ref) writes to a page freed by the unref path.
- Cache dma_addr before xa_cmpxchg; move dma_addr/DMA index
  cleanup to page_pool_return_netmem() which holds a page ref.
- Rename page_pool_release_dma_index() to
  page_pool_remove_dma_mapping() to reflect its new role as a
  pure cmpxchg wrapper.
- Link to v1: https://lore.kernel.org/r/20260724092135.414699-1-shaojijie@huawei.com
---
 net/core/page_pool.c | 46 ++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 21dc4a9c8714..497bb1906fc3 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -500,29 +500,40 @@ static int page_pool_register_dma_index(struct page_pool *pool,
 	return err;
 }
 
-static int page_pool_release_dma_index(struct page_pool *pool,
-				       netmem_ref netmem)
+static void __page_pool_unmap_netmem_dma(struct page_pool *pool,
+					 netmem_ref netmem)
 {
 	struct page *old, *page = netmem_to_page(netmem);
 	unsigned long id;
+	dma_addr_t dma;
+
+	if (!pool->dma_map)
+		return;
+
+	/* Cache dma_addr before xa_cmpxchg. The scrub path holds no page ref;
+	 * the unref path calls put_page() regardless of cmpxchg outcome, so
+	 * after the cmpxchg we cannot safely touch netmem fields.
+	 */
+	dma = page_pool_get_dma_addr_netmem(netmem);
 
 	if (unlikely(!PP_DMA_INDEX_BITS))
-		return 0;
+		goto unmap;
 
 	id = netmem_get_dma_index(netmem);
 	if (!id)
-		return -1;
+		return;
 
 	if (in_softirq())
 		old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
 	else
 		old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
 	if (old != page)
-		return -1;
-
-	netmem_set_dma_index(netmem, 0);
+		return;
 
-	return 0;
+unmap:
+	dma_unmap_page_attrs(pool->p.dev, dma,
+			     PAGE_SIZE << pool->p.order, pool->p.dma_dir,
+			     DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
 }
 
 static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)
@@ -728,24 +739,13 @@ void page_pool_clear_pp_info(netmem_ref netmem)
 static __always_inline void __page_pool_release_netmem_dma(struct page_pool *pool,
 							   netmem_ref netmem)
 {
-	dma_addr_t dma;
-
 	if (!pool->dma_map)
-		/* Always account for inflight pages, even if we didn't
-		 * map them
-		 */
 		return;
 
-	if (page_pool_release_dma_index(pool, netmem))
-		return;
-
-	dma = page_pool_get_dma_addr_netmem(netmem);
-
-	/* When page is unmapped, it cannot be returned to our pool */
-	dma_unmap_page_attrs(pool->p.dev, dma,
-			     PAGE_SIZE << pool->p.order, pool->p.dma_dir,
-			     DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
+	__page_pool_unmap_netmem_dma(pool, netmem);
 	page_pool_set_dma_addr_netmem(netmem, 0);
+	if (likely(PP_DMA_INDEX_BITS))
+		netmem_set_dma_index(netmem, 0);
 }
 
 /* Disconnects a page (from a page_pool).  API users can have a need
@@ -1172,7 +1172,7 @@ static void page_pool_scrub(struct page_pool *pool)
 		}
 
 		xa_for_each(&pool->dma_mapped, id, ptr)
-			__page_pool_release_netmem_dma(pool, page_to_netmem((struct page *)ptr));
+			__page_pool_unmap_netmem_dma(pool, page_to_netmem((struct page *)ptr));
 	}
 
 	/* No more consumers should exist, but producers could still

base-commit: 2195424c3da2ef1829a63b807e3a900a90e57d85
-- 
2.33.0

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Mina Almasry]

On Fri, Jul 31, 2026 at 4:15 AM Jijie Shao <shaojijie@huawei.com> wrote:
>
> page_pool_scrub() iterates pool->dma_mapped via xa_for_each() with no
> page ref held. __page_pool_release_netmem_dma() currently reads and
> writes netmem fields (dma_addr, DMA index bits in pp_magic) after
> xa_cmpxchg() returns. The unref path calls put_page() unconditionally
> regardless of the cmpxchg outcome; when it loses the cmpxchg, it still
> frees the page before the scrub winner finishes these netmem accesses,
> so scrub touches a freed page -- a Use-After-Free.
>
> Fix this by splitting the DMA release into two functions:
>
> 1. __page_pool_unmap_netmem_dma() caches dma_addr before xa_cmpxchg(),
>    does the cmpxchg to remove the DMA mapping, and calls dma_unmap on
>    the cached address. It never touches netmem fields after the cmpxchg,
>    making it safe for the scrub path which holds no page ref.
>
> 2. __page_pool_release_netmem_dma() wraps the above and additionally
>    clears dma_addr and DMA index bits in netmem fields. This is safe
>    only when the caller holds a page ref, so it is used by the return
>    path (page_pool_return_netmem).
>
> The scrub path calls __page_pool_unmap_netmem_dma() directly; the return
> path calls __page_pool_release_netmem_dma().
>
> Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
> Suggested-by: Mina Almasry <almasrymina@google.com>
> Assisted-by: OhMyOpenCode:GLM-5.2
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

Gemini provided no feedback and to my eye the code looks great too.

Reviewed-by: Mina Almasry <almasrymina@google.com>

> ---
> Changes in v4:
> - Restructure per Mina's review: merge page_pool_remove_dma_mapping()
>   into __page_pool_unmap_netmem_dma() with dma_unmap inlined via goto
>   label; simplify __page_pool_release_netmem_dma() to a thin wrapper.
> - Link to v3: https://lore.kernel.org/r/20260729110249.2824835-1-shaojijie@huawei.com
>
> Changes in v3:
> - Fix unlikely() to likely() for PP_DMA_INDEX_BITS to match
>   file convention.
> - Link to v2: https://lore.kernel.org/r/20260727132612.3277927-1-shaojijie@huawei.com
>
> Changes in v2:
> - Redesign the fix per Mina's review: v1's unconditional
>   netmem_set_dma_index() introduced a UAF when the scrub path
>   (no page ref) writes to a page freed by the unref path.
> - Cache dma_addr before xa_cmpxchg; move dma_addr/DMA index
>   cleanup to page_pool_return_netmem() which holds a page ref.
> - Rename page_pool_release_dma_index() to
>   page_pool_remove_dma_mapping() to reflect its new role as a
>   pure cmpxchg wrapper.
> - Link to v1: https://lore.kernel.org/r/20260724092135.414699-1-shaojijie@huawei.com
> ---
>  net/core/page_pool.c | 46 ++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 21dc4a9c8714..497bb1906fc3 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -500,29 +500,40 @@ static int page_pool_register_dma_index(struct page_pool *pool,
>         return err;
>  }
>
> -static int page_pool_release_dma_index(struct page_pool *pool,
> -                                      netmem_ref netmem)
> +static void __page_pool_unmap_netmem_dma(struct page_pool *pool,
> +                                        netmem_ref netmem)
>  {
>         struct page *old, *page = netmem_to_page(netmem);
>         unsigned long id;
> +       dma_addr_t dma;
> +
> +       if (!pool->dma_map)
> +               return;
> +
> +       /* Cache dma_addr before xa_cmpxchg. The scrub path holds no page ref;
> +        * the unref path calls put_page() regardless of cmpxchg outcome, so
> +        * after the cmpxchg we cannot safely touch netmem fields.
> +        */
> +       dma = page_pool_get_dma_addr_netmem(netmem);
>
>         if (unlikely(!PP_DMA_INDEX_BITS))
> -               return 0;
> +               goto unmap;
>
>         id = netmem_get_dma_index(netmem);
>         if (!id)
> -               return -1;
> +               return;
>
>         if (in_softirq())
>                 old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
>         else
>                 old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
>         if (old != page)
> -               return -1;
> -
> -       netmem_set_dma_index(netmem, 0);
> +               return;
>
> -       return 0;
> +unmap:
> +       dma_unmap_page_attrs(pool->p.dev, dma,
> +                            PAGE_SIZE << pool->p.order, pool->p.dma_dir,
> +                            DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
>  }
>
>  static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)
> @@ -728,24 +739,13 @@ void page_pool_clear_pp_info(netmem_ref netmem)
>  static __always_inline void __page_pool_release_netmem_dma(struct page_pool *pool,
>                                                            netmem_ref netmem)
>  {
> -       dma_addr_t dma;
> -
>         if (!pool->dma_map)
> -               /* Always account for inflight pages, even if we didn't
> -                * map them
> -                */
>                 return;
>
> -       if (page_pool_release_dma_index(pool, netmem))
> -               return;
> -
> -       dma = page_pool_get_dma_addr_netmem(netmem);
> -
> -       /* When page is unmapped, it cannot be returned to our pool */
> -       dma_unmap_page_attrs(pool->p.dev, dma,
> -                            PAGE_SIZE << pool->p.order, pool->p.dma_dir,
> -                            DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
> +       __page_pool_unmap_netmem_dma(pool, netmem);
>         page_pool_set_dma_addr_netmem(netmem, 0);
> +       if (likely(PP_DMA_INDEX_BITS))
> +               netmem_set_dma_index(netmem, 0);

I now notice that maybe another cleanup we could have done is open
code __page_pool_unmap_netmem_dma() in this function to cut down 1
helper, and just have the scrub function call
__page_pool_release_netmem_dma() to reduce some code. But this is more
than fine too I think, especially since this is a fix the stable trees
are going to want I guess.

-- 
Thanks,
Mina

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Jakub Kicinski]

On Fri, 31 Jul 2026 10:37:58 -0700 Mina Almasry wrote:
> > +       __page_pool_unmap_netmem_dma(pool, netmem);
> >         page_pool_set_dma_addr_netmem(netmem, 0);
> > +       if (likely(PP_DMA_INDEX_BITS))
> > +               netmem_set_dma_index(netmem, 0);  
> 
> I now notice that maybe another cleanup we could have done is open
> code __page_pool_unmap_netmem_dma() in this function to cut down 1
> helper, and just have the scrub function call
> __page_pool_release_netmem_dma() to reduce some code. But this is more
> than fine too I think, especially since this is a fix the stable trees
> are going to want I guess.

Not sure this is a good idea? scrub is trying to touch just the DMA
mapping, right? It shouldn't try to update the page itself because
it has no reference to the page, the page may get freed in parallel.
Hopefully DMA unmap on a freed page is legal..

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Jakub Kicinski]

On Fri, 31 Jul 2026 19:15:07 +0800 Jijie Shao wrote:
> page_pool_scrub() iterates pool->dma_mapped via xa_for_each() with no
> page ref held. __page_pool_release_netmem_dma() currently reads and
> writes netmem fields (dma_addr, DMA index bits in pp_magic) after
> xa_cmpxchg() returns. The unref path calls put_page() unconditionally
> regardless of the cmpxchg outcome; when it loses the cmpxchg, it still
> frees the page before the scrub winner finishes these netmem accesses,
> so scrub touches a freed page -- a Use-After-Free.
> 
> Fix this by splitting the DMA release into two functions:
> 
> 1. __page_pool_unmap_netmem_dma() caches dma_addr before xa_cmpxchg(),
>    does the cmpxchg to remove the DMA mapping, and calls dma_unmap on
>    the cached address. It never touches netmem fields after the cmpxchg,
>    making it safe for the scrub path which holds no page ref.
> 
> 2. __page_pool_release_netmem_dma() wraps the above and additionally
>    clears dma_addr and DMA index bits in netmem fields. This is safe
>    only when the caller holds a page ref, so it is used by the return
>    path (page_pool_return_netmem).
> 
> The scrub path calls __page_pool_unmap_netmem_dma() directly; the return
> path calls __page_pool_release_netmem_dma().

Please clearly state what led you to discovering this bug?
Was it directly hit in production?
Was there a prod issue which made you investigate?
Were you able to trigger the race and if so -- how?

> Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
> Suggested-by: Mina Almasry <almasrymina@google.com>
> Assisted-by: OhMyOpenCode:GLM-5.2
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
> Changes in v4:
> - Restructure per Mina's review: merge page_pool_remove_dma_mapping()
>   into __page_pool_unmap_netmem_dma() with dma_unmap inlined via goto
>   label; simplify __page_pool_release_netmem_dma() to a thin wrapper.
> - Link to v3: https://lore.kernel.org/r/20260729110249.2824835-1-shaojijie@huawei.com
> 
> Changes in v3:
> - Fix unlikely() to likely() for PP_DMA_INDEX_BITS to match
>   file convention.
> - Link to v2: https://lore.kernel.org/r/20260727132612.3277927-1-shaojijie@huawei.com
> 
> Changes in v2:
> - Redesign the fix per Mina's review: v1's unconditional
>   netmem_set_dma_index() introduced a UAF when the scrub path
>   (no page ref) writes to a page freed by the unref path.
> - Cache dma_addr before xa_cmpxchg; move dma_addr/DMA index
>   cleanup to page_pool_return_netmem() which holds a page ref.
> - Rename page_pool_release_dma_index() to
>   page_pool_remove_dma_mapping() to reflect its new role as a
>   pure cmpxchg wrapper.
> - Link to v1: https://lore.kernel.org/r/20260724092135.414699-1-shaojijie@huawei.com
> ---
>  net/core/page_pool.c | 46 ++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 21dc4a9c8714..497bb1906fc3 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -500,29 +500,40 @@ static int page_pool_register_dma_index(struct page_pool *pool,
>  	return err;
>  }
>  
> -static int page_pool_release_dma_index(struct page_pool *pool,
> -				       netmem_ref netmem)
> +static void __page_pool_unmap_netmem_dma(struct page_pool *pool,
> +					 netmem_ref netmem)
>  {
>  	struct page *old, *page = netmem_to_page(netmem);
>  	unsigned long id;
> +	dma_addr_t dma;
> +
> +	if (!pool->dma_map)
> +		return;
> +
> +	/* Cache dma_addr before xa_cmpxchg. The scrub path holds no page ref;
> +	 * the unref path calls put_page() regardless of cmpxchg outcome, so
> +	 * after the cmpxchg we cannot safely touch netmem fields.
> +	 */
> +	dma = page_pool_get_dma_addr_netmem(netmem);
>  
>  	if (unlikely(!PP_DMA_INDEX_BITS))
> -		return 0;
> +		goto unmap;

nit: just indent the intervening lines please.
Don't use goto where adding a code block would do.

>  	id = netmem_get_dma_index(netmem);
>  	if (!id)
> -		return -1;
> +		return;
>  
>  	if (in_softirq())
>  		old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
>  	else
>  		old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
>  	if (old != page)
> -		return -1;
> -
> -	netmem_set_dma_index(netmem, 0);
> +		return;
>  
> -	return 0;
> +unmap:
> +	dma_unmap_page_attrs(pool->p.dev, dma,
> +			     PAGE_SIZE << pool->p.order, pool->p.dma_dir,
> +			     DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
>  }
-- 
pw-bot: cr

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Mina Almasry]

On Tue, Aug 4, 2026 at 7:16 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 31 Jul 2026 10:37:58 -0700 Mina Almasry wrote:
> > > +       __page_pool_unmap_netmem_dma(pool, netmem);
> > >         page_pool_set_dma_addr_netmem(netmem, 0);
> > > +       if (likely(PP_DMA_INDEX_BITS))
> > > +               netmem_set_dma_index(netmem, 0);
> >
> > I now notice that maybe another cleanup we could have done is open
> > code __page_pool_unmap_netmem_dma() in this function to cut down 1
> > helper, and just have the scrub function call
> > __page_pool_release_netmem_dma() to reduce some code. But this is more
> > than fine too I think, especially since this is a fix the stable trees
> > are going to want I guess.
>
> Not sure this is a good idea? scrub is trying to touch just the DMA
> mapping, right? It shouldn't try to update the page itself because
> it has no reference to the page, the page may get freed in parallel.
> Hopefully DMA unmap on a freed page is legal..

Scrub actually frees the page in the loop in
page_pool_empty_ring(pool), so it's not true AFAIU that it 'shouldn't
try to update the page'.

My mental model (roughly) is that the pp has a ref and that single
last ref can be dropped in the page_pool_destroy() path or the
page_pool_put_netmem() path, but the ref can't be dropped twice and
the same goes for dma unmapping.

Now that i look closer at the code I have no idea why we didn't 'just'
put the dma-unmapping inside of page_pool_return_netmem(). That code
path is already common between scrub() and put_netmem() and does the
synchronization between these 2 paths.

When I have time I'll take a look to see if an LLM can find a better
way to do this.

--
Thanks,
Mina

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Jijie Shao]

on 2026/8/5 11:50, Mina Almasry wrote:
> On Tue, Aug 4, 2026 at 7:16 PM Jakub Kicinski <kuba@kernel.org> wrote:
>> On Fri, 31 Jul 2026 10:37:58 -0700 Mina Almasry wrote:
>>>> +       __page_pool_unmap_netmem_dma(pool, netmem);
>>>>          page_pool_set_dma_addr_netmem(netmem, 0);
>>>> +       if (likely(PP_DMA_INDEX_BITS))
>>>> +               netmem_set_dma_index(netmem, 0);
>>> I now notice that maybe another cleanup we could have done is open
>>> code __page_pool_unmap_netmem_dma() in this function to cut down 1
>>> helper, and just have the scrub function call
>>> __page_pool_release_netmem_dma() to reduce some code. But this is more
>>> than fine too I think, especially since this is a fix the stable trees
>>> are going to want I guess.
>> Not sure this is a good idea? scrub is trying to touch just the DMA
>> mapping, right? It shouldn't try to update the page itself because
>> it has no reference to the page, the page may get freed in parallel.
>> Hopefully DMA unmap on a freed page is legal..
> Scrub actually frees the page in the loop in
> page_pool_empty_ring(pool), so it's not true AFAIU that it 'shouldn't
> try to update the page'.
>
> My mental model (roughly) is that the pp has a ref and that single
> last ref can be dropped in the page_pool_destroy() path or the
> page_pool_put_netmem() path, but the ref can't be dropped twice and
> the same goes for dma unmapping.
>
> Now that i look closer at the code I have no idea why we didn't 'just'
> put the dma-unmapping inside of page_pool_return_netmem(). That code
> path is already common between scrub() and put_netmem() and does the
> synchronization between these 2 paths.
>
> When I have time I'll take a look to see if an LLM can find a better
> way to do this.

Hi Mina,

Thanks for the Reviewed-by.

I'll send v5 shortly addressing Jakub's requests. The deeper
restructure you're musing about seems orthogonal to this fix,
so let's discuss it separately.

Thanks,
Jijie Shao

Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

[Jijie Shao]

on 2026/8/5 10:17, Jakub Kicinski wrote:
> On Fri, 31 Jul 2026 19:15:07 +0800 Jijie Shao wrote:
>> page_pool_scrub() iterates pool->dma_mapped via xa_for_each() with no
>> page ref held. __page_pool_release_netmem_dma() currently reads and
>> writes netmem fields (dma_addr, DMA index bits in pp_magic) after
>> xa_cmpxchg() returns. The unref path calls put_page() unconditionally
>> regardless of the cmpxchg outcome; when it loses the cmpxchg, it still
>> frees the page before the scrub winner finishes these netmem accesses,
>> so scrub touches a freed page -- a Use-After-Free.
>>
>> Fix this by splitting the DMA release into two functions:
>>
>> 1. __page_pool_unmap_netmem_dma() caches dma_addr before xa_cmpxchg(),
>>     does the cmpxchg to remove the DMA mapping, and calls dma_unmap on
>>     the cached address. It never touches netmem fields after the cmpxchg,
>>     making it safe for the scrub path which holds no page ref.
>>
>> 2. __page_pool_release_netmem_dma() wraps the above and additionally
>>     clears dma_addr and DMA index bits in netmem fields. This is safe
>>     only when the caller holds a page ref, so it is used by the return
>>     path (page_pool_return_netmem).
>>
>> The scrub path calls __page_pool_unmap_netmem_dma() directly; the return
>> path calls __page_pool_release_netmem_dma().
> Please clearly state what led you to discovering this bug?
> Was it directly hit in production?
> Was there a prod issue which made you investigate?
> Were you able to trigger the race and if so -- how?
>
>> Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
>> Suggested-by: Mina Almasry <almasrymina@google.com>
>> Assisted-by: OhMyOpenCode:GLM-5.2
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>> ---
>> Changes in v4:
>> - Restructure per Mina's review: merge page_pool_remove_dma_mapping()
>>    into __page_pool_unmap_netmem_dma() with dma_unmap inlined via goto
>>    label; simplify __page_pool_release_netmem_dma() to a thin wrapper.
>> - Link to v3: https://lore.kernel.org/r/20260729110249.2824835-1-shaojijie@huawei.com
>>
>> Changes in v3:
>> - Fix unlikely() to likely() for PP_DMA_INDEX_BITS to match
>>    file convention.
>> - Link to v2: https://lore.kernel.org/r/20260727132612.3277927-1-shaojijie@huawei.com
>>
>> Changes in v2:
>> - Redesign the fix per Mina's review: v1's unconditional
>>    netmem_set_dma_index() introduced a UAF when the scrub path
>>    (no page ref) writes to a page freed by the unref path.
>> - Cache dma_addr before xa_cmpxchg; move dma_addr/DMA index
>>    cleanup to page_pool_return_netmem() which holds a page ref.
>> - Rename page_pool_release_dma_index() to
>>    page_pool_remove_dma_mapping() to reflect its new role as a
>>    pure cmpxchg wrapper.
>> - Link to v1: https://lore.kernel.org/r/20260724092135.414699-1-shaojijie@huawei.com
>> ---
>>   net/core/page_pool.c | 46 ++++++++++++++++++++++----------------------
>>   1 file changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index 21dc4a9c8714..497bb1906fc3 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -500,29 +500,40 @@ static int page_pool_register_dma_index(struct page_pool *pool,
>>   	return err;
>>   }
>>   
>> -static int page_pool_release_dma_index(struct page_pool *pool,
>> -				       netmem_ref netmem)
>> +static void __page_pool_unmap_netmem_dma(struct page_pool *pool,
>> +					 netmem_ref netmem)
>>   {
>>   	struct page *old, *page = netmem_to_page(netmem);
>>   	unsigned long id;
>> +	dma_addr_t dma;
>> +
>> +	if (!pool->dma_map)
>> +		return;
>> +
>> +	/* Cache dma_addr before xa_cmpxchg. The scrub path holds no page ref;
>> +	 * the unref path calls put_page() regardless of cmpxchg outcome, so
>> +	 * after the cmpxchg we cannot safely touch netmem fields.
>> +	 */
>> +	dma = page_pool_get_dma_addr_netmem(netmem);
>>   
>>   	if (unlikely(!PP_DMA_INDEX_BITS))
>> -		return 0;
>> +		goto unmap;
> nit: just indent the intervening lines please.
> Don't use goto where adding a code block would do.
>

Hi Jakub,

Thanks for the review. v5 incoming addressing both points:

- Added bug discovery context to the commit  message.
- Replaced goto with if (likely(PP_DMA_INDEX_BITS)) block.

Jijie