[PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race

Jijie Shao posted 1 patch 12 hours ago
net/core/page_pool.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
[PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race
Posted by Jijie Shao 12 hours ago
page_pool_release_dma_index() uses xa_cmpxchg() to atomically remove a
page from pool->dma_mapped.  When the cmpxchg loses the race (the entry
was already removed by a concurrent release path, e.g. page_pool_scrub()
during page_pool_destroy()), the function returns -1 *without* clearing
the DMA index bits stored in pp_magic.  Its caller then skips dma_unmap
based on that -1, which is correct, but the stale DMA index bits left
in pp_magic are wrong: the xarray entry they index no longer exists.
A page recycled back into the pool then carries a dangling DMA index,
and kernels that validate pp_magic in the return path surface this as
a WARN.

Move netmem_set_dma_index(netmem, 0) to execute unconditionally before
return, so both the winner and the loser of the xa_cmpxchg race clear
the DMA index bits.  The return value still distinguishes the two
cases (-1 loser / 0 winner), preserving the contract that exactly one
side performs dma_unmap.

The race window opens when page_pool_destroy() runs concurrently with
late page returns from SKBs held in per-CPU defer lists, TCP receive
queues or GRO hashes -- e.g. a driver reconfiguring channels/ring
depth while traffic is flowing.

Observed on arm64 (7.2.0-rc1) with the hns3 driver:

  WARNING: net/core/netmem_priv.h:18 at page_pool_clear_pp_info+0x20/0x38,
           CPU#83: iperf/3874878

Return path (CPU_B, process context, triggering the WARN).  The page
was held in a per-CPU SKB defer list and is being released through
tcp_recvmsg():

  page_pool_clear_pp_info+0x20/0x38
  page_pool_put_unrefed_netmem+0x11c/0x2e8
  napi_pp_put_page+0xf0/0x120
  skb_release_data+0x170/0x228
  skb_attempt_defer_free+0x7c/0x1f0
  tcp_recvmsg_locked+0x710/0x9a0
  tcp_recvmsg+0x74/0x1c8
  inet_recvmsg+0x2c/0xf0
  __sys_recvfrom+0xdc/0x198
  __arm64_sys_recvfrom+0x2c/0x48
  invoke_syscall+0x5c/0x120
  el0_svc_common.constprop.0+0xc8/0xf0
  do_el0_svc+0x24/0x38
  el0_svc+0x34/0x1e0
  el0t_64_sync_handler+0xa0/0xe8
  el0t_64_sync+0x1ac/0x1b0

Scrub path (CPU_A, racing with the return path, derived from code):

  page_pool_destroy
  -> page_pool_scrub
     -> xa_for_each(dma_mapped)
        -> __page_pool_release_netmem_dma
           -> page_pool_release_dma_index   <- race window

Debug approach and evidence:
Reproduced with a stability test that repeatedly reconfigures the hns3
channel count and ring depth (ethtool -L/-G) while running iperf3 with
multiple parallel streams, then closes the iperf3 sockets to release
the SKBs held in TCP receive queues and per-CPU defer lists.

On the unmodified kernel the WARN reproduces after hours to tens of
hours. To make the race window observable, a debugfs knob injects a
controlled udelay() right after the winning xa_cmpxchg() in
page_pool_release_dma_index(), widening the gap between the xarray
entry removal and the pp_magic cleanup.  With the delay injected the
WARN reproduces 4-5 times within about half an hour.

Instrumentation added to page_pool_release_dma_index() logs the per-
page pp_magic, DMA index, refcount and dma_addr at release time.  A
representative log line from a WARN-triggering page:

  PP_DMA_IDX: magic=0xdead0000000cbec0 idx=6525 refcnt=1 dma_addr=0x0

  magic=0xdead0000000cbec0  -> PP_SIGNATURE | (6525 << shift)
  idx=6525                  -> DMA index bits still set in pp_magic
  dma_addr=0x0              -> scrub already did dma_unmap + cleared addr
  refcnt=1                  -> inflight page, unrefed release path

The combination "dma_addr == 0 but DMA index != 0" is the signature of
the race: CPU_A (scrub) won the xa_cmpxchg, performed dma_unmap and
cleared dma_addr; CPU_B (return) lost the cmpxchg, returned -1 and
skipped netmem_set_dma_index(0), leaving the DMA index bits stale --
exactly the gap this fix closes.

With this fix applied the WARN no longer triggers under the same
workload (verified overnight), both with and without the debugfs
delay knob enabled.

Fixes: 95920c2ed02b ("page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches")
Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
Assisted-by: OhMyOpenCode:GLM-5.2
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 net/core/page_pool.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 21dc4a9c8714..6f86c96650d3 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -505,6 +505,7 @@ static int page_pool_release_dma_index(struct page_pool *pool,
 {
 	struct page *old, *page = netmem_to_page(netmem);
 	unsigned long id;
+	int ret = 0;
 
 	if (unlikely(!PP_DMA_INDEX_BITS))
 		return 0;
@@ -517,12 +518,18 @@ static int page_pool_release_dma_index(struct page_pool *pool,
 		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;
+	if (old != page) {
+		/* xarray entry already removed by concurrent release path
+		 * (e.g. page_pool_scrub). Still clear DMA index bits to
+		 * keep pp_magic consistent. The winner of the xa_cmpxchg
+		 * race is responsible for dma_unmap.
+		 */
+		ret = -1;
+	}
 
 	netmem_set_dma_index(netmem, 0);
 
-	return 0;
+	return ret;
 }
 
 static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)

base-commit: 78f75d632f74b8de0f081a128588f7c37d0d1164
-- 
2.33.0
Re: [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race
Posted by Mina Almasry 2 hours ago
On Fri, Jul 24, 2026 at 2:22 AM Jijie Shao <shaojijie@huawei.com> wrote:
>
> page_pool_release_dma_index() uses xa_cmpxchg() to atomically remove a
> page from pool->dma_mapped.  When the cmpxchg loses the race (the entry
> was already removed by a concurrent release path, e.g. page_pool_scrub()
> during page_pool_destroy()), the function returns -1 *without* clearing
> the DMA index bits stored in pp_magic.  Its caller then skips dma_unmap
> based on that -1, which is correct, but the stale DMA index bits left
> in pp_magic are wrong: the xarray entry they index no longer exists.
> A page recycled back into the pool then carries a dangling DMA index,
> and kernels that validate pp_magic in the return path surface this as
> a WARN.
>
> Move netmem_set_dma_index(netmem, 0) to execute unconditionally before
> return, so both the winner and the loser of the xa_cmpxchg race clear
> the DMA index bits.  The return value still distinguishes the two
> cases (-1 loser / 0 winner), preserving the contract that exactly one
> side performs dma_unmap.
>
> The race window opens when page_pool_destroy() runs concurrently with
> late page returns from SKBs held in per-CPU defer lists, TCP receive
> queues or GRO hashes -- e.g. a driver reconfiguring channels/ring
> depth while traffic is flowing.
>
> Observed on arm64 (7.2.0-rc1) with the hns3 driver:
>
>   WARNING: net/core/netmem_priv.h:18 at page_pool_clear_pp_info+0x20/0x38,
>            CPU#83: iperf/3874878
>
> Return path (CPU_B, process context, triggering the WARN).  The page
> was held in a per-CPU SKB defer list and is being released through
> tcp_recvmsg():
>
>   page_pool_clear_pp_info+0x20/0x38
>   page_pool_put_unrefed_netmem+0x11c/0x2e8
>   napi_pp_put_page+0xf0/0x120
>   skb_release_data+0x170/0x228
>   skb_attempt_defer_free+0x7c/0x1f0
>   tcp_recvmsg_locked+0x710/0x9a0
>   tcp_recvmsg+0x74/0x1c8
>   inet_recvmsg+0x2c/0xf0
>   __sys_recvfrom+0xdc/0x198
>   __arm64_sys_recvfrom+0x2c/0x48
>   invoke_syscall+0x5c/0x120
>   el0_svc_common.constprop.0+0xc8/0xf0
>   do_el0_svc+0x24/0x38
>   el0_svc+0x34/0x1e0
>   el0t_64_sync_handler+0xa0/0xe8
>   el0t_64_sync+0x1ac/0x1b0
>
> Scrub path (CPU_A, racing with the return path, derived from code):
>
>   page_pool_destroy
>   -> page_pool_scrub
>      -> xa_for_each(dma_mapped)
>         -> __page_pool_release_netmem_dma
>            -> page_pool_release_dma_index   <- race window
>
> Debug approach and evidence:
> Reproduced with a stability test that repeatedly reconfigures the hns3
> channel count and ring depth (ethtool -L/-G) while running iperf3 with
> multiple parallel streams, then closes the iperf3 sockets to release
> the SKBs held in TCP receive queues and per-CPU defer lists.
>
> On the unmodified kernel the WARN reproduces after hours to tens of
> hours. To make the race window observable, a debugfs knob injects a
> controlled udelay() right after the winning xa_cmpxchg() in
> page_pool_release_dma_index(), widening the gap between the xarray
> entry removal and the pp_magic cleanup.  With the delay injected the
> WARN reproduces 4-5 times within about half an hour.
>
> Instrumentation added to page_pool_release_dma_index() logs the per-
> page pp_magic, DMA index, refcount and dma_addr at release time.  A
> representative log line from a WARN-triggering page:
>
>   PP_DMA_IDX: magic=0xdead0000000cbec0 idx=6525 refcnt=1 dma_addr=0x0
>
>   magic=0xdead0000000cbec0  -> PP_SIGNATURE | (6525 << shift)
>   idx=6525                  -> DMA index bits still set in pp_magic
>   dma_addr=0x0              -> scrub already did dma_unmap + cleared addr
>   refcnt=1                  -> inflight page, unrefed release path
>
> The combination "dma_addr == 0 but DMA index != 0" is the signature of
> the race: CPU_A (scrub) won the xa_cmpxchg, performed dma_unmap and
> cleared dma_addr; CPU_B (return) lost the cmpxchg, returned -1 and
> skipped netmem_set_dma_index(0), leaving the DMA index bits stale --
> exactly the gap this fix closes.
>
> With this fix applied the WARN no longer triggers under the same
> workload (verified overnight), both with and without the debugfs
> delay knob enabled.
>
> Fixes: 95920c2ed02b ("page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches")
> Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
> Assisted-by: OhMyOpenCode:GLM-5.2
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

If at all possible, when generating fixes via LLMs, spend time
reviewing the patch yourself before sending to make sure the patch
makes sense. In this case, I was taken back that this 10+, 3- fix came
with such a huge huge commit message.

I did not review my self but my LLM - which does not tire in reading
the long commit message - came up with this review. It looks like it
may be correct. I think the fact that we do not touch the netmem at
all if we lose the race is intentional, because the netmem may not be
allocated:

```
  While this fixes the WARN_ON_ONCE, it introduces a critical
Use-After-Free (UAF) and memory corruption bug in the scrub path.

  page_pool_scrub() iterates over pool->dma_mapped without holding a
reference to the page. If the scrub path races with the normal unref
path and wins xa_cmpxchg(), the unref path will proceed to free the
page.

  If the scrub path is preempted immediately after xa_cmpxchg(), the
page will be freed before the scrub path can continue. When the scrub
path resumes, it will execute your new netmem_set_dma_index(netmem,
0), read the dma_addr,
  and clear it—all on a freed page that may now belong to another
subsystem. This will corrupt memory and unmap garbage DMA addresses.

  To safely fix this:

  1. __page_pool_release_netmem_dma() must cache dma_addr to a local
variable before calling xa_cmpxchg().
  2. The scrub path (winner) must not touch netmem after a successful
xa_cmpxchg().
  3. The unref path (loser), which safely holds the page reference,
should handle clearing the DMA index and dma_addr to satisfy the WARN,
regardless of the race outcome.
```

-- 
Thanks,
Mina