[Devel] [PATCH hci-8.1] ms/net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

* [Devel] [PATCH hci-8.1] ms/net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race
@ 2026-10-22 15:04 Konstantin Khorenko
  2026-10-22 15:08 ` Konstantin Khorenko
  0 siblings, 1 reply; 2+ messages in thread
From: Konstantin Khorenko @ 2026-10-22 15:04 UTC (permalink / raw)
  To: devel, Konstantin Khorenko

From: Jijie Shao <shaojijie@huawei.com>

This bug was discovered while testing the hns3 driver under channel
reconfiguration (`ethtool -L` / `ethtool -G`) with iperf3 traffic on
arm64. The race is intermittently triggered when page_pool_destroy()
runs page_pool_scrub() concurrently with page return via
page_pool_put_netmem() on a different CPU. A WARN in
page_pool_clear_pp_info() surfaced the dangling DMA index bits left
by the cmpxchg loser, which led to the investigation.

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>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://patch.msgid.link/20260807114830.344336-1-shaojijie@huawei.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 24ef02f934eeb48830cff6b739abc3c62b1d107b)

Rebase note: this tree lacks upstream 4ad125ae380b ("page_pool: rename
__page_pool_release_page_dma() to __page_pool_release_netmem_dma()"),
so the return-path helper keeps its old name and page_pool_scrub()
passes ptr without the (struct page *) cast. The code is otherwise the
same as the mainline commit and the 6.12.y backport bbfef303f980.

Found while reviewing the PP_MAGIC_MASK backport for VSTOR-148112: the
scrub path clears dma_addr and the DMA index of a page the return path
may already have freed, which on x86_64 corrupts the struct page of a
reused page (lru.next and index).

https://virtuozzo.atlassian.net/browse/VSTOR-148112

Feature: fix ms/net/page_pool
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Follow-up to the PP_MAGIC_MASK backport for VSTOR-148112: the same
upstream DMA index tracking has a use-after-free in page_pool_scrub(),
fixed in mainline 24ef02f934ee and in 6.12.y as bbfef303f980.
CentOS Stream 10 does not have it yet (checked up to 6.12.0-279.el10).

Assisted-by: LLM
---
 net/core/page_pool.c | 66 ++++++++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 58f07690dc312..a61107f1dbf96 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -490,29 +490,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 (unlikely(!PP_DMA_INDEX_BITS))
-		return 0;
-
-	id = netmem_get_dma_index(netmem);
-	if (!id)
-		return -1;
+	if (!pool->dma_map)
+		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;
+	/* 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);
 
-	netmem_set_dma_index(netmem, 0);
+	if (likely(PP_DMA_INDEX_BITS)) {
+		id = netmem_get_dma_index(netmem);
+		if (!id)
+			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;
+	}
 
-	return 0;
+	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)
@@ -713,24 +724,16 @@ void page_pool_clear_pp_info(netmem_ref netmem)
 static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
 							 netmem_ref netmem)
 {
-	dma_addr_t dma;
-
+	/* Caller must hold a page ref: __page_pool_unmap_netmem_dma() is
+	 * safe without a ref, but the field clears below require it.
+	 */
 	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
@@ -1157,8 +1160,9 @@ static void page_pool_scrub(struct page_pool *pool)
 				synchronize_net();
 		}
 
+		/* No page ref, dma-unmap only. */
 		xa_for_each(&pool->dma_mapped, id, ptr)
-			__page_pool_release_page_dma(pool, page_to_netmem(ptr));
+			__page_pool_unmap_netmem_dma(pool, page_to_netmem(ptr));
 	}
 
 	/* No more consumers should exist, but producers could still

---
base-commit: 2517011d6bef38fa3e4a71e044fef9493ed6a34b
change-id: 20261022-khorenko-vstor-148112-page_pool-scrub-uaf-143e37108662

Best regards,
--  
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 2+ messages in thread