* [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race
@ 2026-07-24 9:21 Jijie Shao
2026-07-24 18:33 ` Mina Almasry
0 siblings, 1 reply; 3+ messages in thread
From: Jijie Shao @ 2026-07-24 9:21 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, andrew+netdev, horms, hawk,
ilias.apalodimas, almasrymina, toke
Cc: shenjian15, liuyonglong, chenhao418, yangshuaisong, netdev,
linux-kernel, shaojijie
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race
2026-07-24 9:21 [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race Jijie Shao
@ 2026-07-24 18:33 ` Mina Almasry
2026-07-25 2:48 ` Jijie Shao
0 siblings, 1 reply; 3+ messages in thread
From: Mina Almasry @ 2026-07-24 18:33 UTC (permalink / raw)
To: Jijie Shao
Cc: davem, edumazet, kuba, pabeni, andrew+netdev, horms, hawk,
ilias.apalodimas, toke, shenjian15, liuyonglong, chenhao418,
yangshuaisong, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race
2026-07-24 18:33 ` Mina Almasry
@ 2026-07-25 2:48 ` Jijie Shao
0 siblings, 0 replies; 3+ messages in thread
From: Jijie Shao @ 2026-07-25 2:48 UTC (permalink / raw)
To: Mina Almasry
Cc: shaojijie, davem, edumazet, kuba, pabeni, andrew+netdev, horms,
hawk, ilias.apalodimas, toke, shenjian15, liuyonglong, chenhao418,
yangshuaisong, netdev, linux-kernel
on 2026/7/25 2:33, Mina Almasry wrote:
> 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.
Thanks for the review, Mina. Fair point on the commit message length — this
was my first time digging into page_pool internals and the message ended up
documenting the whole investigation path. I first saw the call trace on
6.17-rc4 and spent a long time chasing the hns3 driver before realizing the
issue was in page_pool itself. The fix was LLM-suggested and I did run it,
but only under iperf — which, as you note, is not enough to surface the
UAF (the freed page isn't reused fast enough for the stale write to land on
a different subsystem's memory, so "no crash / no WARN" lulled me into
thinking the fix was safe). I should have reasoned about the refcount
contract from the code rather than relying on workload symptoms. I'll trim
the commit message heavily in v2.
>
> 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.
You're right. I verified in the source:
- page_pool_scrub() walks pool->dma_mapped via xa_for_each() with no
get_page on the entries (net/core/page_pool.c:1178).
- page_pool_return_netmem() calls put_page() after
__page_pool_release_netmem_dma() returns (net/core/page_pool.c:778),
so the unref path can free the page once refcount hits 0.
So "loser doesn't touch netmem" is indeed the intended contract, and v1
breaking it on the scrub-loser path is a real UAF.
One clarification on the scope of the LLM review's UAF claim: the scrub
**winner** UAF it describes is pre-existing, not introduced by v1. In
__page_pool_release_netmem_dma() (unchanged by v1), the winner path does:
if (page_pool_release_dma_index(pool, netmem)) // cmpxchg
return;
dma = page_pool_get_dma_addr_netmem(netmem); // READ netmem->dma_addr
dma_unmap_page_attrs(..., dma, ...);
page_pool_set_dma_addr_netmem(netmem, 0); // WRITE netmem->dma_addr
These three netmem dereferences happen after the cmpxchg wins, with no page
ref held. If the unref loser races to put_page() in between, the scrub winner
reads/writes a freed page. v1 doesn't change the winner path; it only adds a
new netmem_set_dma_index() write on the **loser** path, which is the new UAF
v1 introduces.
>
> 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.
For v2 I'll follow your direction:
1. Cache dma_addr to a local variable before xa_cmpxchg() in
__page_pool_release_netmem_dma().
2. Scrub path (winner or loser) does not touch netmem fields after
cmpxchg; it only does dma_unmap on the cached address.
3. Unref path, which holds the page ref, clears dma_addr and DMA index
bits regardless of the cmpxchg outcome.
This fixes both the v1-introduced loser UAF and the pre-existing winner UAF.
Will send v2 soon.
Thanks,
Jijie Shao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-25 2:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 9:21 [PATCH net] net: page_pool: fix DMA index not cleared on xa_cmpxchg race Jijie Shao
2026-07-24 18:33 ` Mina Almasry
2026-07-25 2:48 ` Jijie Shao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox