[Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Eva Kurchatova]

Helge reported that the introduction of PP_MAGIC_MASK let to crashes on
boot on his 32-bit parisc machine. The cause of this is the mask is set
too wide, so the page_pool_page_is_pp() incurs false positives which
crashes the machine.

Just disabling the check in page_pool_is_pp() will lead to the page_pool
code itself malfunctioning; so instead of doing this, this patch changes
the define for PP_DMA_INDEX_BITS to avoid mistaking arbitrary kernel
pointers for page_pool-tagged pages.

The fix relies on the kernel pointers that alias with the pp_magic field
always being above PAGE_OFFSET. With this assumption, we can use the
lowest bit of the value of PAGE_OFFSET as the upper bound of the
PP_DMA_INDEX_MASK, which should avoid the false positives.

Because we cannot rely on PAGE_OFFSET always being a compile-time
constant, nor on it always being >0, we fall back to disabling the
dma_index storage when there are not enough bits available. This leaves
us in the situation we were in before the patch in the Fixes tag, but
only on a subset of architecture configurations. This seems to be the
best we can do until the transition to page types in complete for
page_pool pages.

v2:
- Make sure there's at least 8 bits available and that the PAGE_OFFSET
  bit calculation doesn't wrap

Link: https://lore.kernel.org/all/aMNJMFa5fDalFmtn@p100/
Fixes: ee62ce7a1d90 ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
Cc: stable@vger.kernel.org # 6.15+
Tested-by: Helge Deller <deller@gmx.de>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Tested-by: Helge Deller <deller@gmx.de>
Link: https://patch.msgid.link/20250930114331.675412-1-toke@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 95920c2ed02bde551ab654e9749c2ca7bc3100e0)

Rebase note: context only, the pp_magic block sits earlier in this
tree's mm.h; the change is the same. On User Mode Linux PAGE_OFFSET is
a variable, so the DMA index tracking goes off there and a kernel
pointer in page->lru.next no longer passes for a page pool page, which
is the "page_pool leak" bad page every KUnit run reported.

https://virtuozzo.atlassian.net/browse/VSTOR-148112
Feature: fix ms/net/page_pool

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 include/linux/mm.h   | 22 +++++++------
 net/core/page_pool.c | 76 ++++++++++++++++++++++++++++++--------------
 2 files changed, 66 insertions(+), 32 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 75b8bcd9f442..c61ae2b3ed7b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -408,14 +408,13 @@ extern unsigned int kobjsize(const void *objp);
  * since this value becomes part of PP_SIGNATURE; meaning we can just use the
  * space between the PP_SIGNATURE value (without POISON_POINTER_DELTA), and the
  * lowest bits of POISON_POINTER_DELTA. On arches where POISON_POINTER_DELTA is
- * 0, we make sure that we leave the two topmost bits empty, as that guarantees
- * we won't mistake a valid kernel pointer for a value we set, regardless of the
- * VMSPLIT setting.
+ * 0, we use the lowest bit of PAGE_OFFSET as the boundary if that value is
+ * known at compile-time.
  *
- * Altogether, this means that the number of bits available is constrained by
- * the size of an unsigned long (at the upper end, subtracting two bits per the
- * above), and the definition of PP_SIGNATURE (with or without
- * POISON_POINTER_DELTA).
+ * If the value of PAGE_OFFSET is not known at compile time, or if it is too
+ * small to leave at least 8 bits available above PP_SIGNATURE, we define the
+ * number of bits to be 0, which turns off the DMA index tracking altogether
+ * (see page_pool_register_dma_index()).
  */
 #define PP_DMA_INDEX_SHIFT (1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA))
 #if POISON_POINTER_DELTA > 0
@@ -424,8 +423,13 @@ extern unsigned int kobjsize(const void *objp);
  */
 #define PP_DMA_INDEX_BITS MIN(32, __ffs(POISON_POINTER_DELTA) - PP_DMA_INDEX_SHIFT)
 #else
-/* Always leave out the topmost two; see above. */
-#define PP_DMA_INDEX_BITS MIN(32, BITS_PER_LONG - PP_DMA_INDEX_SHIFT - 2)
+/* Use the lowest bit of PAGE_OFFSET if there's at least 8 bits available; see above */
+#define PP_DMA_INDEX_MIN_OFFSET (1 << (PP_DMA_INDEX_SHIFT + 8))
+#define PP_DMA_INDEX_BITS ((__builtin_constant_p(PAGE_OFFSET) && \
+			    PAGE_OFFSET >= PP_DMA_INDEX_MIN_OFFSET && \
+			    !(PAGE_OFFSET & (PP_DMA_INDEX_MIN_OFFSET - 1))) ? \
+			      MIN(32, __ffs(PAGE_OFFSET) - PP_DMA_INDEX_SHIFT) : 0)
+
 #endif
 
 #define PP_DMA_INDEX_MASK GENMASK(PP_DMA_INDEX_BITS + PP_DMA_INDEX_SHIFT - 1, \
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 15b08364d246..58f07690dc31 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -465,11 +465,60 @@ page_pool_dma_sync_for_device(const struct page_pool *pool,
 	}
 }
 
+static int page_pool_register_dma_index(struct page_pool *pool,
+					netmem_ref netmem, gfp_t gfp)
+{
+	int err = 0;
+	u32 id;
+
+	if (unlikely(!PP_DMA_INDEX_BITS))
+		goto out;
+
+	if (in_softirq())
+		err = xa_alloc(&pool->dma_mapped, &id, netmem_to_page(netmem),
+			       PP_DMA_INDEX_LIMIT, gfp);
+	else
+		err = xa_alloc_bh(&pool->dma_mapped, &id, netmem_to_page(netmem),
+				  PP_DMA_INDEX_LIMIT, gfp);
+	if (err) {
+		WARN_ONCE(err != -ENOMEM, "couldn't track DMA mapping, please report to netdev@");
+		goto out;
+	}
+
+	netmem_set_dma_index(netmem, id);
+out:
+	return err;
+}
+
+static int page_pool_release_dma_index(struct page_pool *pool,
+				       netmem_ref netmem)
+{
+	struct page *old, *page = netmem_to_page(netmem);
+	unsigned long id;
+
+	if (unlikely(!PP_DMA_INDEX_BITS))
+		return 0;
+
+	id = netmem_get_dma_index(netmem);
+	if (!id)
+		return -1;
+
+	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 0;
+}
+
 static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)
 {
 	dma_addr_t dma;
 	int err;
-	u32 id;
 
 	/* Setup DMA mapping: use 'struct page' area for storing DMA-addr
 	 * since dma_addr_t can be either 32 or 64 bits and does not always fit
@@ -488,18 +537,10 @@ static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t g
 		goto unmap_failed;
 	}
 
-	if (in_softirq())
-		err = xa_alloc(&pool->dma_mapped, &id, netmem_to_page(netmem),
-			       PP_DMA_INDEX_LIMIT, gfp);
-	else
-		err = xa_alloc_bh(&pool->dma_mapped, &id, netmem_to_page(netmem),
-				  PP_DMA_INDEX_LIMIT, gfp);
-	if (err) {
-		WARN_ONCE(err != -ENOMEM, "couldn't track DMA mapping, please report to netdev@");
+	err = page_pool_register_dma_index(pool, netmem, gfp);
+	if (err)
 		goto unset_failed;
-	}
 
-	netmem_set_dma_index(netmem, id);
 	page_pool_dma_sync_for_device(pool, netmem, pool->p.max_len);
 
 	return true;
@@ -672,8 +713,6 @@ 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)
 {
-	struct page *old, *page = netmem_to_page(netmem);
-	unsigned long id;
 	dma_addr_t dma;
 
 	if (!pool->dma_map)
@@ -682,15 +721,7 @@ static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
 		 */
 		return;
 
-	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)
+	if (page_pool_release_dma_index(pool, netmem))
 		return;
 
 	dma = page_pool_get_dma_addr_netmem(netmem);
@@ -700,7 +731,6 @@ static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
 			     PAGE_SIZE << pool->p.order, pool->p.dma_dir,
 			     DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
 	page_pool_set_dma_addr_netmem(netmem, 0);
-	netmem_set_dma_index(netmem, 0);
 }
 
 /* Disconnects a page (from a page_pool).  API users can have a need
-- 
2.56.0

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

Re: [Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: PP_DMA_INDEX_MASK uses an inverted GENMASK range when PP_DMA_INDEX_BITS folds to 0
--

commit 430b5a3ba30b9045b484a6671db44819d46c0d9d
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

This backport changes PP_DMA_INDEX_BITS so it is derived from the lowest
set bit of PAGE_OFFSET. When PAGE_OFFSET is not a usable compile-time
constant, the bit count falls back to 0 and DMA index tracking is turned
off. The DMA index register and release code in page_pool.c is moved into
helper functions.

Link: https://lore.kernel.org/all/aMNJMFa5fDalFmtn@p100/
Link: https://patch.msgid.link/20250930114331.675412-1-toke@redhat.com

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h

[ ... ]

>  #define PP_DMA_INDEX_SHIFT (1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA))
>  #if POISON_POINTER_DELTA > 0

[ ... ]

>  #else
> -/* Always leave out the topmost two; see above. */
> -#define PP_DMA_INDEX_BITS MIN(32, BITS_PER_LONG - PP_DMA_INDEX_SHIFT - 2)
> +/* Use the lowest bit of PAGE_OFFSET if there's at least 8 bits available; see above */
> +#define PP_DMA_INDEX_MIN_OFFSET (1 << (PP_DMA_INDEX_SHIFT + 8))
> +#define PP_DMA_INDEX_BITS ((__builtin_constant_p(PAGE_OFFSET) && \
> +			    PAGE_OFFSET >= PP_DMA_INDEX_MIN_OFFSET && \
> +			    !(PAGE_OFFSET & (PP_DMA_INDEX_MIN_OFFSET - 1))) ? \
> +			      MIN(32, __ffs(PAGE_OFFSET) - PP_DMA_INDEX_SHIFT) : 0)
> +
>  #endif
>  
>  #define PP_DMA_INDEX_MASK GENMASK(PP_DMA_INDEX_BITS + PP_DMA_INDEX_SHIFT - 1, \

[Severity: Medium]
Can PP_DMA_INDEX_MASK break the build when PP_DMA_INDEX_BITS folds to 0?

PP_DMA_INDEX_MASK is unchanged, so with zero bits it expands to:

    GENMASK(0 + PP_DMA_INDEX_SHIFT - 1, PP_DMA_INDEX_SHIFT)

That makes h = SHIFT-1 and l = SHIFT, which is an inverted range.

GENMASK() is defined as:

    GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)

GENMASK_INPUT_CHECK() is BUILD_BUG_ON_ZERO(const_true((l) > (h))), and
const_true() is true whenever the expression is an integer constant
expression.

On architectures where PAGE_OFFSET is a compile-time constant that is
too small or has low bits set, every operand of the ternary in
PP_DMA_INDEX_BITS is constant. Two examples are MIPS Malta, which has
"#define PAGE_OFFSET _AC(0x0, UL)", and m68k, which has
"PAGE_OFFSET_RAW 0x00000000". NOMMU configs that take PAGE_OFFSET from
CONFIG_ values may also hit this.

In those cases the bit count should fold to a constant 0, and "l > h"
would then be a constant true. BUILD_BUG_ON_ZERO would fire in every
translation unit that expands PP_MAGIC_MASK or PP_DMA_INDEX_MASK. That
includes net/core/page_pool.c, the netmem_priv.h users, and
page_pool_page_is_pp() under CONFIG_PAGE_POOL.

The UML case described in the rebase note is not affected, because
PAGE_OFFSET is a variable there. __builtin_constant_p() is not an ICE
in that case, so const_true() evaluates to false and the runtime mask is
0.

I could not run a compiler, so whether GCC treats this ternary as an
integer constant expression is reasoned from the code and not observed.

The new comment and the commit message describe 0 bits as the general
fallback, but the mask definition was not updated to handle it. Should
PP_DMA_INDEX_MASK be defined as 0 when PP_DMA_INDEX_BITS is 0, or use a
form that tolerates an empty range?

The upstream commit 95920c2ed02b defines the mask the same way, so this
is not specific to the backport.

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261021225127.2383437-1-eva.kurchatova@virtuozzo.com?part=1

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

Re: [Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Eva Kurchatova (Virtuozzo)]

On 10/22/26 01:57, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm: PP_DMA_INDEX_MASK uses an inverted GENMASK range when PP_DMA_INDEX_BITS folds to 0
> --
>
> commit 430b5a3ba30b9045b484a6671db44819d46c0d9d
> Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>
> ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches
>
> This backport changes PP_DMA_INDEX_BITS so it is derived from the lowest
> set bit of PAGE_OFFSET. When PAGE_OFFSET is not a usable compile-time
> constant, the bit count falls back to 0 and DMA index tracking is turned
> off. The DMA index register and release code in page_pool.c is moved into
> helper functions.
>
> Link: https://lore.kernel.org/all/aMNJMFa5fDalFmtn@p100/
> Link: https://patch.msgid.link/20250930114331.675412-1-toke@redhat.com
>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
> [ ... ]
>
>>   #define PP_DMA_INDEX_SHIFT (1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA))
>>   #if POISON_POINTER_DELTA > 0
> [ ... ]
>
>>   #else
>> -/* Always leave out the topmost two; see above. */
>> -#define PP_DMA_INDEX_BITS MIN(32, BITS_PER_LONG - PP_DMA_INDEX_SHIFT - 2)
>> +/* Use the lowest bit of PAGE_OFFSET if there's at least 8 bits available; see above */
>> +#define PP_DMA_INDEX_MIN_OFFSET (1 << (PP_DMA_INDEX_SHIFT + 8))
>> +#define PP_DMA_INDEX_BITS ((__builtin_constant_p(PAGE_OFFSET) && \
>> +			    PAGE_OFFSET >= PP_DMA_INDEX_MIN_OFFSET && \
>> +			    !(PAGE_OFFSET & (PP_DMA_INDEX_MIN_OFFSET - 1))) ? \
>> +			      MIN(32, __ffs(PAGE_OFFSET) - PP_DMA_INDEX_SHIFT) : 0)
>> +
>>   #endif
>>   
>>   #define PP_DMA_INDEX_MASK GENMASK(PP_DMA_INDEX_BITS + PP_DMA_INDEX_SHIFT - 1, \
> [Severity: Medium]
> Can PP_DMA_INDEX_MASK break the build when PP_DMA_INDEX_BITS folds to 0?
>
> PP_DMA_INDEX_MASK is unchanged, so with zero bits it expands to:
>
>      GENMASK(0 + PP_DMA_INDEX_SHIFT - 1, PP_DMA_INDEX_SHIFT)
>
> That makes h = SHIFT-1 and l = SHIFT, which is an inverted range.
>
> GENMASK() is defined as:
>
>      GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l)
>
> GENMASK_INPUT_CHECK() is BUILD_BUG_ON_ZERO(const_true((l) > (h))), and
> const_true() is true whenever the expression is an integer constant
> expression.
>
> On architectures where PAGE_OFFSET is a compile-time constant that is
> too small or has low bits set, every operand of the ternary in
> PP_DMA_INDEX_BITS is constant. Two examples are MIPS Malta, which has
> "#define PAGE_OFFSET _AC(0x0, UL)", and m68k, which has
> "PAGE_OFFSET_RAW 0x00000000". NOMMU configs that take PAGE_OFFSET from
> CONFIG_ values may also hit this.
>
> In those cases the bit count should fold to a constant 0, and "l > h"
> would then be a constant true. BUILD_BUG_ON_ZERO would fire in every
> translation unit that expands PP_MAGIC_MASK or PP_DMA_INDEX_MASK. That
> includes net/core/page_pool.c, the netmem_priv.h users, and
> page_pool_page_is_pp() under CONFIG_PAGE_POOL.
>
> The UML case described in the rebase note is not affected, because
> PAGE_OFFSET is a variable there. __builtin_constant_p() is not an ICE
> in that case, so const_true() evaluates to false and the runtime mask is
> 0.
>
> I could not run a compiler, so whether GCC treats this ternary as an
> integer constant expression is reasoned from the code and not observed.
>
> The new comment and the commit message describe 0 bits as the general
> fallback, but the mask definition was not updated to handle it. Should
> PP_DMA_INDEX_MASK be defined as 0 when PP_DMA_INDEX_BITS is 0, or use a
> form that tolerates an empty range?
>
> The upstream commit 95920c2ed02b defines the mask the same way, so this
> is not specific to the backport.
>
> [ ... ]

Checked, this is not a problem in practice and there isn't a follow-up 
upstream fix,

because it isn't needed. __GENMASK() folds to zero on its own in this case.

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

Re: [Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Konstantin Khorenko]

2 more ms fixes on this commit:

  ┌──────────────┬──────────────┬──────────────────────────────────────────────────────────────┐
  │   mainline   │    6.12.y    │                     what is fixed                            │
  ├──────────────┼──────────────┼──────────────────────────────────────────────────────────────┤
  │ 24ef02f934ee │ bbfef303f980 │ fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race │
  ├──────────────┼──────────────┼──────────────────────────────────────────────────────────────┤
  │ 039f248a6cc1 │ d471b17ad549 │ Remove zone/policy GFP flags when allocating XArray entries  │
  └──────────────┴──────────────┴──────────────────────────────────────────────────────────────┘

Do we need them as well?

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

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

Re: [Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Konstantin Khorenko]

On Thu, 22 Oct 2026 01:51:24 +0300, Eva Kurchatova wrote:
> ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.10.vz10, thanks!

[1/1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/2517011d6bef

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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

Re: [Devel] [PATCH hci-8.1] ms/page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches

[Konstantin Khorenko]

On 10/22/26 13:31, Konstantin Khorenko wrote:
> 2 more ms fixes on this commit:
> 
>   ┌──────────────┬──────────────┬──────────────────────────────────────────────────────────────┐
>   │   mainline   │    6.12.y    │                     what is fixed                            │
>   ├──────────────┼──────────────┼──────────────────────────────────────────────────────────────┤
>   │ 24ef02f934ee │ bbfef303f980 │ fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race │
>   ├──────────────┼──────────────┼──────────────────────────────────────────────────────────────┤
>   │ 039f248a6cc1 │ d471b17ad549 │ Remove zone/policy GFP flags when allocating XArray entries  │
>   └──────────────┴──────────────┴──────────────────────────────────────────────────────────────┘
> 
> Do we need them as well?
answering to myself.
1st patch is needed, i'll take it.
2nd patch is not needed.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel