* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
2026-10-13 12:30 [PATCH] mm: fix address wraparound when scanning large folio PTEs David Hildenbrand (Arm)
@ 2026-10-13 12:57 ` Kiryl Shutsemau
2026-10-13 13:17 ` Barry Song
2026-10-13 14:11 ` Lorenzo Stoakes (ARM)
1 sibling, 1 reply; 4+ messages in thread
From: Kiryl Shutsemau @ 2026-10-13 12:57 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Ying Huang,
Baolin Wang, Barry Song, Ryan Roberts, Chuanhua Han, Kyle Zeng,
linux-mm, linux-kernel, stable
On Tue, Oct 13, 2026 at 02:30:37PM +0200, David Hildenbrand (Arm) wrote:
> numa_rebuild_large_mapping() and do_swap_page() can be tricked into
> generating an address wraparound, crashing the kernel. All that's required
> is to map a tail page of a large folio close enough to the start/end of
> the address space to e.g., make
>
> unsigned long addr_start = addr - (nr << PAGE_SHIFT);
>
> to wrap around.
>
> Let's fix it by providing a better helper, folio_pte_scan_range(), that
> takes care of staying within VMA and page table boundaries while
> handling possible wraparounds.
>
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Fixes: d2136d749d76 ("mm: support multi-size THP numa balancing")
> Fixes: 508758960b8d ("mm: swap: entirely map large folios found in swapcache")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
> mm/memory.c | 43 +++++++++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 1f5d5f7d39cd2..a3823010aa981 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4933,6 +4933,27 @@ static void check_swap_exclusive(struct folio *folio, swp_entry_t entry,
> } while (--nr_pages);
> }
>
> +/*
> + * Get the address range in which a folio might be mapped, assuming its pages
> + * are mapped consecutively and the folio page at the given idx is mapped at
> + * the given address.
> + */
> +static void folio_pte_scan_range(const struct folio *folio,
> + const struct vm_area_struct *vma, unsigned long addr,
> + unsigned long idx, unsigned long *start, unsigned long *end)
> +{
> + /* Stay within the page table and VMA boundaries. */
> + const unsigned long pmd_start = addr & PMD_MASK;
> + const unsigned long pmd_end = pmd_start + PMD_SIZE;
> + const unsigned long lower = max(pmd_start, vma->vm_start);
> + const unsigned long upper = min(pmd_end - 1, vma->vm_end - 1) + 1;
These -1/+1 gave me a pause. I guess it is for pmd_end wrapping to 0 in
the top PMD? A comment would be nice.
Otherwise:
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
2026-10-13 12:30 [PATCH] mm: fix address wraparound when scanning large folio PTEs David Hildenbrand (Arm)
2026-10-13 12:57 ` Kiryl Shutsemau
@ 2026-10-13 14:11 ` Lorenzo Stoakes (ARM)
1 sibling, 0 replies; 4+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-13 14:11 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Ying Huang, Baolin Wang,
Barry Song, Ryan Roberts, Chuanhua Han, Kiryl Shutsemau,
Kyle Zeng, linux-mm, linux-kernel, stable
On Tue, Oct 13, 2026 at 02:30:37PM +0200, David Hildenbrand (Arm) wrote:
> numa_rebuild_large_mapping() and do_swap_page() can be tricked into
> generating an address wraparound, crashing the kernel. All that's required
> is to map a tail page of a large folio close enough to the start/end of
> the address space to e.g., make
>
> unsigned long addr_start = addr - (nr << PAGE_SHIFT);
>
> to wrap around.
Yeah damn.
I feel like we need to generalise the underflow/overflow checks we have
arbitrarily around the place into helper functions.
I know somebody schlopped something like that for one function in particular
recently, but a non-schlopped, generalised version of that would be nice.
>
> Let's fix it by providing a better helper, folio_pte_scan_range(), that
> takes care of staying within VMA and page table boundaries while
> handling possible wraparounds.
OK so this is effectively abstracting this very thing :) though I guess
perhaps not quite a match for the general 'did we over/underflow stuff with
the addr, size pair we just got'.
Anyway that's something to think about separately I guess.
>
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Fixes: d2136d749d76 ("mm: support multi-size THP numa balancing")
> Fixes: 508758960b8d ("mm: swap: entirely map large folios found in swapcache")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
> mm/memory.c | 43 +++++++++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 1f5d5f7d39cd2..a3823010aa981 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4933,6 +4933,27 @@ static void check_swap_exclusive(struct folio *folio, swp_entry_t entry,
> } while (--nr_pages);
> }
>
> +/*
> + * Get the address range in which a folio might be mapped, assuming its pages
> + * are mapped consecutively and the folio page at the given idx is mapped at
> + * the given address.
> + */
I know it's a static function but kdoc here might be nice given the number of
params, including 2 output parameters.
Also then can specify exclusive end, etc.
> +static void folio_pte_scan_range(const struct folio *folio,
> + const struct vm_area_struct *vma, unsigned long addr,
> + unsigned long idx, unsigned long *start, unsigned long *end)
I don't love the name - to me it implies to me that you're scanning the range.
Maybe clamp_folio_pte_range()? I guess though clamp implies it limits an input
range.
Maybe get_folio_pte_range()?
Also I think it might be useful to have it return the number of pages contained
within the range?
Then this below:
+ folio_pte_scan_range(folio, vma, address, idx, &folio_start,
+ &folio_end);
+ if (unlikely((folio_end - folio_start) >> PAGE_SHIFT != nr))
Could be:
+ if (unlikely(get_folio_pte_range(folio, vma, address, idx,
+ &folio_start, &folio_end)) != nr)
OTOH, that's pretty ugly... :))
> +{
> + /* Stay within the page table and VMA boundaries. */
> + const unsigned long pmd_start = addr & PMD_MASK;
> + const unsigned long pmd_end = pmd_start + PMD_SIZE;
OK so get the range of the PTE page table.
> + const unsigned long lower = max(pmd_start, vma->vm_start);
Then clamp to VMA start...
> + const unsigned long upper = min(pmd_end - 1, vma->vm_end - 1) + 1;
This is trickier :)
I guess you're doing this in case either bound is right at wraparound
somehow?
But then couldn't the +1 push you over the edge?
Oh wait no, because then vma->vm_end would have to equal 0, which would
fail the other-place enforced invariant that vma->vm_start < vma->vm_end.
So it's pmd_end that could overflow, hence the -1, +1 dance.
Yeah I kind of hate that, that's a lot of 'just have to know' right there.
Actually we have pmd_addr_end() already, could we use that?
#define pmd_addr_end(addr, end) \
({ unsigned long __boundary = ((addr) + PMD_SIZE) & PMD_MASK; \
(__boundary - 1 < (end) - 1)? __boundary: (end); \
})
That embeds the trick in the helper and it expresses what you're getting.
Then it could be:
const unsigned long pmd_start = addr & PMD_MASK;
const unsigned long lower = max(pmd_start, vma->vm_start);
const unsigned long upper = pmd_addr_end(addr, vma->vm_end);
> + const unsigned long offset = idx << PAGE_SHIFT;
> +
> + /* Avoid any address wraparounds. */
> + *start = addr - min(addr - lower, offset);
This is also tricky. This is clamping to max(lower, addr - offset) right,
but being very careful about underflow?
I don't love the addr - (addr - lower) = lower here, and it feels like it
embeds a comparison there too. It's all very implicit.
Actually maybe just naming addr - lower gets us there:
const unsigned long max_offset = addr - lower;
...
*start = addr - min(max_offset, offset);
There's check_sub_overflow() or you could use a ternary also but both feel
like they add more complexity than is useful.
> + *end = addr + min(upper - addr, folio_size(folio) - offset);
Same deal here.
Maybe something like:
const unsigned long max_remaining_bytes = upper - addr;
...
*end = addr + min(max_remaining_bytes, folio_size(folio) - offset);
> +}
> +
> /*
> * We enter with either the VMA lock or the mmap_lock held (see
> * FAULT_FLAG_VMA_LOCK), and pte mapped but not yet locked.
> @@ -5121,16 +5142,15 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> address = vmf->address;
> ptep = vmf->pte;
> if (folio_test_large(folio) && folio_test_swapcache(folio)) {
> - int nr = folio_nr_pages(folio);
> - unsigned long idx = folio_page_idx(folio, page);
> - unsigned long folio_start = address - idx * PAGE_SIZE;
> - unsigned long folio_end = folio_start + nr * PAGE_SIZE;
> + const int nr = folio_nr_pages(folio);
> + const unsigned long idx = folio_page_idx(folio, page);
> + unsigned long folio_start, folio_end;
> pte_t *folio_ptep;
> pte_t folio_pte;
>
> - if (unlikely(folio_start < max(address & PMD_MASK, vma->vm_start)))
> - goto check_folio;
> - if (unlikely(folio_end > pmd_addr_end(address, vma->vm_end)))
> + folio_pte_scan_range(folio, vma, address, idx, &folio_start,
> + &folio_end);
> + if (unlikely((folio_end - folio_start) >> PAGE_SHIFT != nr))
> goto check_folio;
>
> folio_ptep = vmf->pte - idx;
> @@ -6280,16 +6300,11 @@ static void numa_rebuild_large_mapping(struct vm_fault *vmf, struct vm_area_stru
> struct folio *folio, pte_t fault_pte,
> bool ignore_writable, bool pte_write_upgrade)
> {
> - int nr = pte_pfn(fault_pte) - folio_pfn(folio);
> + const unsigned long idx = pte_pfn(fault_pte) - folio_pfn(folio);
> unsigned long start, end, addr = vmf->address;
> - unsigned long addr_start = addr - (nr << PAGE_SHIFT);
> - unsigned long pt_start = ALIGN_DOWN(addr, PMD_SIZE);
> pte_t *start_ptep;
>
> - /* Stay within the VMA and within the page table. */
> - start = max3(addr_start, pt_start, vma->vm_start);
> - end = min3(addr_start + folio_size(folio), pt_start + PMD_SIZE,
> - vma->vm_end);
> + folio_pte_scan_range(folio, vma, addr, idx, &start, &end);
> start_ptep = vmf->pte - ((addr - start) >> PAGE_SHIFT);
>
> /* Restore all PTEs' mapping of the large folio */
>
> ---
>
> base-commit: 634cbf5b794b734c78631b92e381cfc85f357bbe
>
> change-id: 20261013-folio_pte_scan_range-c6ca19cc8e9d
>
> --
>
> Cheers,
>
> David
>
In general though the logic as a whole looks correct to me, AFAICT.
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 4+ messages in thread