Skip to content

Commit 15b8a5b

Browse files
tohojogregkh
authored andcommitted
page_pool: Fix PP_MAGIC_MASK to avoid crashing on some 32-bit arches
commit 95920c2 upstream. 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: ee62ce7 ("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> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 68a8fc3 commit 15b8a5b

File tree

2 files changed

+66
-32
lines changed

2 files changed

+66
-32
lines changed

include/linux/mm.h

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4318,14 +4318,13 @@ static inline void pgalloc_tag_copy(struct folio *new, struct folio *old)
43184318
* since this value becomes part of PP_SIGNATURE; meaning we can just use the
43194319
* space between the PP_SIGNATURE value (without POISON_POINTER_DELTA), and the
43204320
* lowest bits of POISON_POINTER_DELTA. On arches where POISON_POINTER_DELTA is
4321-
* 0, we make sure that we leave the two topmost bits empty, as that guarantees
4322-
* we won't mistake a valid kernel pointer for a value we set, regardless of the
4323-
* VMSPLIT setting.
4321+
* 0, we use the lowest bit of PAGE_OFFSET as the boundary if that value is
4322+
* known at compile-time.
43244323
*
4325-
* Altogether, this means that the number of bits available is constrained by
4326-
* the size of an unsigned long (at the upper end, subtracting two bits per the
4327-
* above), and the definition of PP_SIGNATURE (with or without
4328-
* POISON_POINTER_DELTA).
4324+
* If the value of PAGE_OFFSET is not known at compile time, or if it is too
4325+
* small to leave at least 8 bits available above PP_SIGNATURE, we define the
4326+
* number of bits to be 0, which turns off the DMA index tracking altogether
4327+
* (see page_pool_register_dma_index()).
43294328
*/
43304329
#define PP_DMA_INDEX_SHIFT (1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA))
43314330
#if POISON_POINTER_DELTA > 0
@@ -4334,8 +4333,13 @@ static inline void pgalloc_tag_copy(struct folio *new, struct folio *old)
43344333
*/
43354334
#define PP_DMA_INDEX_BITS MIN(32, __ffs(POISON_POINTER_DELTA) - PP_DMA_INDEX_SHIFT)
43364335
#else
4337-
/* Always leave out the topmost two; see above. */
4338-
#define PP_DMA_INDEX_BITS MIN(32, BITS_PER_LONG - PP_DMA_INDEX_SHIFT - 2)
4336+
/* Use the lowest bit of PAGE_OFFSET if there's at least 8 bits available; see above */
4337+
#define PP_DMA_INDEX_MIN_OFFSET (1 << (PP_DMA_INDEX_SHIFT + 8))
4338+
#define PP_DMA_INDEX_BITS ((__builtin_constant_p(PAGE_OFFSET) && \
4339+
PAGE_OFFSET >= PP_DMA_INDEX_MIN_OFFSET && \
4340+
!(PAGE_OFFSET & (PP_DMA_INDEX_MIN_OFFSET - 1))) ? \
4341+
MIN(32, __ffs(PAGE_OFFSET) - PP_DMA_INDEX_SHIFT) : 0)
4342+
43394343
#endif
43404344

43414345
#define PP_DMA_INDEX_MASK GENMASK(PP_DMA_INDEX_BITS + PP_DMA_INDEX_SHIFT - 1, \

net/core/page_pool.c

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,60 @@ page_pool_dma_sync_for_device(const struct page_pool *pool,
462462
}
463463
}
464464

465+
static int page_pool_register_dma_index(struct page_pool *pool,
466+
netmem_ref netmem, gfp_t gfp)
467+
{
468+
int err = 0;
469+
u32 id;
470+
471+
if (unlikely(!PP_DMA_INDEX_BITS))
472+
goto out;
473+
474+
if (in_softirq())
475+
err = xa_alloc(&pool->dma_mapped, &id, netmem_to_page(netmem),
476+
PP_DMA_INDEX_LIMIT, gfp);
477+
else
478+
err = xa_alloc_bh(&pool->dma_mapped, &id, netmem_to_page(netmem),
479+
PP_DMA_INDEX_LIMIT, gfp);
480+
if (err) {
481+
WARN_ONCE(err != -ENOMEM, "couldn't track DMA mapping, please report to netdev@");
482+
goto out;
483+
}
484+
485+
netmem_set_dma_index(netmem, id);
486+
out:
487+
return err;
488+
}
489+
490+
static int page_pool_release_dma_index(struct page_pool *pool,
491+
netmem_ref netmem)
492+
{
493+
struct page *old, *page = netmem_to_page(netmem);
494+
unsigned long id;
495+
496+
if (unlikely(!PP_DMA_INDEX_BITS))
497+
return 0;
498+
499+
id = netmem_get_dma_index(netmem);
500+
if (!id)
501+
return -1;
502+
503+
if (in_softirq())
504+
old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
505+
else
506+
old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
507+
if (old != page)
508+
return -1;
509+
510+
netmem_set_dma_index(netmem, 0);
511+
512+
return 0;
513+
}
514+
465515
static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)
466516
{
467517
dma_addr_t dma;
468518
int err;
469-
u32 id;
470519

471520
/* Setup DMA mapping: use 'struct page' area for storing DMA-addr
472521
* since dma_addr_t can be either 32 or 64 bits and does not always fit
@@ -485,18 +534,10 @@ static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t g
485534
goto unmap_failed;
486535
}
487536

488-
if (in_softirq())
489-
err = xa_alloc(&pool->dma_mapped, &id, netmem_to_page(netmem),
490-
PP_DMA_INDEX_LIMIT, gfp);
491-
else
492-
err = xa_alloc_bh(&pool->dma_mapped, &id, netmem_to_page(netmem),
493-
PP_DMA_INDEX_LIMIT, gfp);
494-
if (err) {
495-
WARN_ONCE(err != -ENOMEM, "couldn't track DMA mapping, please report to netdev@");
537+
err = page_pool_register_dma_index(pool, netmem, gfp);
538+
if (err)
496539
goto unset_failed;
497-
}
498540

499-
netmem_set_dma_index(netmem, id);
500541
page_pool_dma_sync_for_device(pool, netmem, pool->p.max_len);
501542

502543
return true;
@@ -669,8 +710,6 @@ void page_pool_clear_pp_info(netmem_ref netmem)
669710
static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
670711
netmem_ref netmem)
671712
{
672-
struct page *old, *page = netmem_to_page(netmem);
673-
unsigned long id;
674713
dma_addr_t dma;
675714

676715
if (!pool->dma_map)
@@ -679,15 +718,7 @@ static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
679718
*/
680719
return;
681720

682-
id = netmem_get_dma_index(netmem);
683-
if (!id)
684-
return;
685-
686-
if (in_softirq())
687-
old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
688-
else
689-
old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
690-
if (old != page)
721+
if (page_pool_release_dma_index(pool, netmem))
691722
return;
692723

693724
dma = page_pool_get_dma_addr_netmem(netmem);
@@ -697,7 +728,6 @@ static __always_inline void __page_pool_release_page_dma(struct page_pool *pool,
697728
PAGE_SIZE << pool->p.order, pool->p.dma_dir,
698729
DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
699730
page_pool_set_dma_addr_netmem(netmem, 0);
700-
netmem_set_dma_index(netmem, 0);
701731
}
702732

703733
/* Disconnects a page (from a page_pool). API users can have a need

0 commit comments

Comments
 (0)