All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: fix address wraparound when scanning large folio PTEs
@ 2026-10-13 12:30 David Hildenbrand (Arm)
  2026-10-13 12:57 ` Kiryl Shutsemau
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-13 12:30 UTC (permalink / raw)
  To: 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,
	Mike Rapoport, Kiryl Shutsemau, Kyle Zeng
  Cc: linux-mm, linux-kernel, stable, David Hildenbrand (Arm)

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;
+	const unsigned long offset = idx << PAGE_SHIFT;
+
+	/* Avoid any address wraparounds. */
+	*start = addr - min(addr - lower, offset);
+	*end = addr + min(upper - addr, 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



^ permalink raw reply related	[flat|nested] 15+ 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 13:17   ` Barry Song
  2026-10-14  8:04   ` David Hildenbrand (Arm)
  2026-10-13 14:11 ` Lorenzo Stoakes (ARM)
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ 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] 15+ messages in thread

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-13 12:57 ` Kiryl Shutsemau
@ 2026-10-13 13:17   ` Barry Song
  2026-10-14  8:03     ` David Hildenbrand (Arm)
  2026-10-14  8:04   ` David Hildenbrand (Arm)
  1 sibling, 1 reply; 15+ messages in thread
From: Barry Song @ 2026-10-13 13:17 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: David Hildenbrand (Arm), Andrew Morton, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Ying Huang, Baolin Wang,
	Ryan Roberts, Chuanhua Han, Kyle Zeng, linux-mm, linux-kernel,
	stable

On Tue, Oct 13, 2026 at 8:57 PM Kiryl Shutsemau <kas@kernel.org> wrote:
>
> 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.

Will pmd_addr_end() make it better? I mean

upper = pmd_addr_end(address, vma->vm_end);

nop?

Thanks
Barry


^ permalink raw reply	[flat|nested] 15+ 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)
  2026-10-14  8:12   ` David Hildenbrand (Arm)
  2026-10-14  6:02 ` Baolin Wang
  2026-10-14 16:30 ` Lance Yang
  3 siblings, 1 reply; 15+ 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] 15+ 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)
@ 2026-10-14  6:02 ` Baolin Wang
  2026-10-14  8:02   ` David Hildenbrand (Arm)
  2026-10-14 16:30 ` Lance Yang
  3 siblings, 1 reply; 15+ messages in thread
From: Baolin Wang @ 2026-10-14  6:02 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Andrew Morton, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, Ying Huang, Barry Song,
	Ryan Roberts, Chuanhua Han, Kiryl Shutsemau, Kyle Zeng
  Cc: linux-mm, linux-kernel, stable



On 10/13/26 8:30 PM, 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>
> ---

Thanks for the fix.

>   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;

I also agree with Barry and Lorenzo's suggestion to use the more 
readable pmd_addr_end() helper. With Lorenzo's comments addressed, feel 
free to add:
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14  6:02 ` Baolin Wang
@ 2026-10-14  8:02   ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14  8:02 UTC (permalink / raw)
  To: Baolin Wang, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Ying Huang, Barry Song, Ryan Roberts, Chuanhua Han,
	Kiryl Shutsemau, Kyle Zeng
  Cc: linux-mm, linux-kernel, stable

On 10/14/26 08:02, Baolin Wang wrote:
> 
> 
> On 10/13/26 8:30 PM, 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>
>> ---
> 
> Thanks for the fix.
> 
>>   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;
> 
> I also agree with Barry and Lorenzo's suggestion to use the more readable
> pmd_addr_end() helper. 

I had that, then I found that for folded page tables it does something rather
odd, which made me assume "this is not better at all".

-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-13 13:17   ` Barry Song
@ 2026-10-14  8:03     ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14  8:03 UTC (permalink / raw)
  To: Barry Song, Kiryl Shutsemau
  Cc: Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
	Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Ying Huang,
	Baolin Wang, Ryan Roberts, Chuanhua Han, Kyle Zeng, linux-mm,
	linux-kernel, stable

On 10/13/26 15:17, Barry Song wrote:
> On Tue, Oct 13, 2026 at 8:57 PM Kiryl Shutsemau <kas@kernel.org> wrote:
>>
>> 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.
> 
> Will pmd_addr_end() make it better? I mean
> 
> upper = pmd_addr_end(address, vma->vm_end);
> 
> nop?
I don't like pmd_addr_end(), given the weird semantics:

include/asm-generic/pgtable-nopmd.h:#define pmd_addr_end(addr, end)                     (end)

Even if that doesn't apply here, I much rather spell out the boundaries clearly.


-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-13 12:57 ` Kiryl Shutsemau
  2026-10-13 13:17   ` Barry Song
@ 2026-10-14  8:04   ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14  8:04 UTC (permalink / raw)
  To: Kiryl Shutsemau
  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 10/13/26 14:57, Kiryl Shutsemau wrote:
> 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.

I'll add a comment if I decide to leave that in as is, thanks!


-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-13 14:11 ` Lorenzo Stoakes (ARM)
@ 2026-10-14  8:12   ` David Hildenbrand (Arm)
  2026-10-14 13:27     ` Lorenzo Stoakes (ARM)
  0 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14  8:12 UTC (permalink / raw)
  To: Lorenzo Stoakes (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

>> 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.

I had that, but decided "not worth it for an internal helper".

Is anything here really that confusing to understand in mm/memory.c context? I
highly doubt it.

> 
> 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.

Ugly.

> 
> Maybe get_folio_pte_range()?

I can just call it "folio_pte_range()". Slightly similar (but different) to
"folio_pte_batch()"

> 
> 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... :))


Not a fan of these semantics.

> 
>> +{
>> +	/* 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.
> 

Yes. It's fairly common, though, to handle clamping of exclusive ends that way.

> 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);                \
> })

As replied elsewhere, I had that, but decided to open-code as pmd_addr_end() has
very weird semantics with folded page tables, where we just don't clamp at all.

I'll sleep over it, but to me using pmd_addr_end() is not particularly better.

> 
> 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);
> 

Sorry, to push back, but are we overthinking this helper here?

I much rather get this fixed quickly then fine tuning some details in a helper
with clear semantics that anybody can understand when looking at it for 1 minute?


-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14  8:12   ` David Hildenbrand (Arm)
@ 2026-10-14 13:27     ` Lorenzo Stoakes (ARM)
  2026-10-14 13:45       ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 15+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-14 13:27 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 Wed, Oct 14, 2026 at 10:12:06AM +0200, David Hildenbrand (Arm) wrote:
> >> 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>

Since I'm convinced the logic itself is correct:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

This cycle is brutal enough not to hold up fixes :>)

> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index 1f5d5f7d39cd2..a3823010aa981 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c

> >> +/*
> >> + * 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.
>
> I had that, but decided "not worth it for an internal helper".
>
> Is anything here really that confusing to understand in mm/memory.c context? I
> highly doubt it.

I mean I found it pretty confusing :)

But this is a nice to have it's not a big deal.

> >> +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.
>
> Ugly.

Yup, fair enough.

>
> >
> > Maybe get_folio_pte_range()?
>
> I can just call it "folio_pte_range()". Slightly similar (but different) to
> "folio_pte_batch()"

OK that works.

> > OTOH, that's pretty ugly... :))
>
>
> Not a fan of these semantics.

Yeah on reflection not so great, I sort of talked myself out of it a bit as I
typed...

>
> >
> >> +{
> >> +	/* 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;
> >
> > 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.
> >
>
> Yes. It's fairly common, though, to handle clamping of exclusive ends that way.

Right, it took me a while to figure this out though.

It involves an implicit assumption in there too, that _one_ of these values is
known not to overflow.

> > #define pmd_addr_end(addr, end)                                         \
> > ({      unsigned long __boundary = ((addr) + PMD_SIZE) & PMD_MASK;      \
> >         (__boundary - 1 < (end) - 1)? __boundary: (end);                \
> > })
>
> As replied elsewhere, I had that, but decided to open-code as pmd_addr_end() has
> very weird semantics with folded page tables, where we just don't clamp at all.

Hmm how so?

The logic seems, at a glance, equivalent?

>
> I'll sleep over it, but to me using pmd_addr_end() is not particularly better.

I'm not going to block things on that basis as we have a bug to fix here, but I
really had to sit down and take time to understand any of this.

> > 	*end = addr + min(max_remaining_bytes, folio_size(folio) - offset);
> >
>
> Sorry, to push back, but are we overthinking this helper here?
>
> I much rather get this fixed quickly then fine tuning some details in a helper
> with clear semantics that anybody can understand when looking at it for 1 minute?

As above, I tagged it, for the purposes of a fix let's just get this in.

But no, there's not overthinking IMO, it took me a long time to understand what
you were doing here, and there's a lot of implicit understanding in there.

And while I don't claim to be a great genius, I am fairly familiar with mm code,
so that's always to me a gauge on complexity.

Anyway, let's get this in, and we can debate the details upstream at our leisure
:)

--
Cheers, Lorenzo


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14 13:27     ` Lorenzo Stoakes (ARM)
@ 2026-10-14 13:45       ` David Hildenbrand (Arm)
  2026-10-14 13:49         ` David Hildenbrand (Arm)
  2026-10-14 14:56         ` Lorenzo Stoakes (ARM)
  0 siblings, 2 replies; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14 13:45 UTC (permalink / raw)
  To: Lorenzo Stoakes (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 10/14/26 15:27, Lorenzo Stoakes (ARM) wrote:
> On Wed, Oct 14, 2026 at 10:12:06AM +0200, David Hildenbrand (Arm) wrote:
> 
> Since I'm convinced the logic itself is correct:
> 
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 
> This cycle is brutal enough not to hold up fixes :>)
> 
> 
>>>
>>> I know it's a static function but kdoc here might be nice given the number of
>>> params, including 2 output parameters.
>>
>> I had that, but decided "not worth it for an internal helper".
>>
>> Is anything here really that confusing to understand in mm/memory.c context? I
>> highly doubt it.
> 
> I mean I found it pretty confusing :)
> 
> But this is a nice to have it's not a big deal.
> 
>>>
>>> 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.
>>
>> Ugly.
> 
> Yup, fair enough.
> 
>>
>>>
>>> Maybe get_folio_pte_range()?
>>
>> I can just call it "folio_pte_range()". Slightly similar (but different) to
>> "folio_pte_batch()"
> 
> OK that works.

Ok, let's do that.

>>>
>>>
>>> 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.
>>>
>>
>> Yes. It's fairly common, though, to handle clamping of exclusive ends that way.
> 
> Right, it took me a while to figure this out though.
> 
> It involves an implicit assumption in there too, that _one_ of these values is
> known not to overflow.
> 
>>> #define pmd_addr_end(addr, end)                                         \
>>> ({      unsigned long __boundary = ((addr) + PMD_SIZE) & PMD_MASK;      \
>>>         (__boundary - 1 < (end) - 1)? __boundary: (end);                \
>>> })
>>
>> As replied elsewhere, I had that, but decided to open-code as pmd_addr_end() has
>> very weird semantics with folded page tables, where we just don't clamp at all.
> 
> Hmm how so?
> 
> The logic seems, at a glance, equivalent?

It's the following thing that adds confusion:

include/asm-generic/pgtable-nopmd.h:#define pmd_addr_end(addr, end)
       (end)

I think we have to assume here that with NOPMD we will never end up in this code
because large folios will not apply (each PMD table has exactly one entry as of
today, so we cannot map large folios .... I think).

I can try condensing that in a comment and cross my fingers that that is the
case. I tried that now. and codex review already tells me:

  • [P1] Folded PMDs can reach this helper — memory.c:4948 (mm/memory.c:4948).
The comment’s assumption is false: arm64 with 64 KiB pages and 42-bit VA uses
folded PMDs and supports THP and
    NUMA balancing. Here, pmd_addr_end() returns vma->vm_end without clamping. A
large folio remapped across a PTE-table boundary can therefore make
numa_rebuild_large_mapping() walk beyond
    the mapped PTE table. The previous code explicitly limited this to pt_start
+ PMD_SIZE. Cap the forward distance at PMD_SIZE - (addr & ~PMD_MASK) as well as
the VMA and folio limits.

Let me see if there is any truth to that ...


[...]

> 
> As above, I tagged it, for the purposes of a fix let's just get this in.
> 
> But no, there's not overthinking IMO, it took me a long time to understand what
> you were doing here, and there's a lot of implicit understanding in there.

What I am saying is that I don't agree with adding another two const variables
with names that are less intuitive than just saying the computation. Let me try
to improve the comment:

/* Avoid address wraparounds by not adding/subtracting beyond limits. */

-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14 13:45       ` David Hildenbrand (Arm)
@ 2026-10-14 13:49         ` David Hildenbrand (Arm)
  2026-10-14 14:57           ` Lorenzo Stoakes (ARM)
  2026-10-14 14:56         ` Lorenzo Stoakes (ARM)
  1 sibling, 1 reply; 15+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-14 13:49 UTC (permalink / raw)
  To: Lorenzo Stoakes (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 10/14/26 15:45, David Hildenbrand (Arm) wrote:
> On 10/14/26 15:27, Lorenzo Stoakes (ARM) wrote:
>> On Wed, Oct 14, 2026 at 10:12:06AM +0200, David Hildenbrand (Arm) wrote:
>>
>> Since I'm convinced the logic itself is correct:
>>
>> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>>
>> This cycle is brutal enough not to hold up fixes :>)
>>
>>
>>>
>>> I had that, but decided "not worth it for an internal helper".
>>>
>>> Is anything here really that confusing to understand in mm/memory.c context? I
>>> highly doubt it.
>>
>> I mean I found it pretty confusing :)
>>
>> But this is a nice to have it's not a big deal.
>>
>>>
>>> Ugly.
>>
>> Yup, fair enough.
>>
>>>
>>>
>>> I can just call it "folio_pte_range()". Slightly similar (but different) to
>>> "folio_pte_batch()"
>>
>> OK that works.
> 
> Ok, let's do that.
> 
>>>
>>> Yes. It's fairly common, though, to handle clamping of exclusive ends that way.
>>
>> Right, it took me a while to figure this out though.
>>
>> It involves an implicit assumption in there too, that _one_ of these values is
>> known not to overflow.
>>
>>>
>>> As replied elsewhere, I had that, but decided to open-code as pmd_addr_end() has
>>> very weird semantics with folded page tables, where we just don't clamp at all.
>>
>> Hmm how so?
>>
>> The logic seems, at a glance, equivalent?
> 
> It's the following thing that adds confusion:
> 
> include/asm-generic/pgtable-nopmd.h:#define pmd_addr_end(addr, end)
>        (end)
> 
> I think we have to assume here that with NOPMD we will never end up in this code
> because large folios will not apply (each PMD table has exactly one entry as of
> today, so we cannot map large folios .... I think).
> 
> I can try condensing that in a comment and cross my fingers that that is the
> case. I tried that now. and codex review already tells me:
> 
>   • [P1] Folded PMDs can reach this helper — memory.c:4948 (mm/memory.c:4948).
> The comment’s assumption is false: arm64 with 64 KiB pages and 42-bit VA uses
> folded PMDs and supports THP and
>     NUMA balancing. Here, pmd_addr_end() returns vma->vm_end without clamping. A
> large folio remapped across a PTE-table boundary can therefore make
> numa_rebuild_large_mapping() walk beyond
>     the mapped PTE table. The previous code explicitly limited this to pt_start
> + PMD_SIZE. Cap the forward distance at PMD_SIZE - (addr & ~PMD_MASK) as well as
> the VMA and folio limits.
> 
> Let me see if there is any truth to that ...

Ok, the PMD table as a single entry, but the PTE table obviously has 2k entries :)

So large folios are possible and pmd_addr_end() would be wrong in this context.

pmd_addr_end() really only works in page table walkers ...

-- 
Cheers,

David


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14 13:45       ` David Hildenbrand (Arm)
  2026-10-14 13:49         ` David Hildenbrand (Arm)
@ 2026-10-14 14:56         ` Lorenzo Stoakes (ARM)
  1 sibling, 0 replies; 15+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-14 14:56 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 Wed, Oct 14, 2026 at 03:45:41PM +0200, David Hildenbrand (Arm) wrote:
> It's the following thing that adds confusion:
>
> include/asm-generic/pgtable-nopmd.h:#define pmd_addr_end(addr, end)
>        (end)

Ah ugh :) of course.

> What I am saying is that I don't agree with adding another two const variables
> with names that are less intuitive than just saying the computation. Let me try
> to improve the comment:
>
> /* Avoid address wraparounds by not adding/subtracting beyond limits. */

The variables are a (possibly) neat way of self-documenting that, but then
possibly adds complexity by having yet another variable :)

A comment works for me, though the confusing bit I guess is knowing which bit
exactly might overflow.

So maybe saying 'avoid wraparounds of <variable that can wraparound>' and a
brief bit about how would help?

--
Cheers, Lorenzo


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

* Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs
  2026-10-14 13:49         ` David Hildenbrand (Arm)
@ 2026-10-14 14:57           ` Lorenzo Stoakes (ARM)
  0 siblings, 0 replies; 15+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-10-14 14:57 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 Wed, Oct 14, 2026 at 03:49:05PM +0200, David Hildenbrand (Arm) wrote:
> > I think we have to assume here that with NOPMD we will never end up in this code
> > because large folios will not apply (each PMD table has exactly one entry as of
> > today, so we cannot map large folios .... I think).
> >
> > I can try condensing that in a comment and cross my fingers that that is the
> > case. I tried that now. and codex review already tells me:
> >
> >   • [P1] Folded PMDs can reach this helper — memory.c:4948 (mm/memory.c:4948).
> > The comment’s assumption is false: arm64 with 64 KiB pages and 42-bit VA uses
> > folded PMDs and supports THP and
> >     NUMA balancing. Here, pmd_addr_end() returns vma->vm_end without clamping. A
> > large folio remapped across a PTE-table boundary can therefore make
> > numa_rebuild_large_mapping() walk beyond
> >     the mapped PTE table. The previous code explicitly limited this to pt_start
> > + PMD_SIZE. Cap the forward distance at PMD_SIZE - (addr & ~PMD_MASK) as well as
> > the VMA and folio limits.
> >
> > Let me see if there is any truth to that ...
>
> Ok, the PMD table as a single entry, but the PTE table obviously has 2k entries :)
>
> So large folios are possible and pmd_addr_end() would be wrong in this context.
>
> pmd_addr_end() really only works in page table walkers ...

Ack!

OK yeah, I missed that it gets redefined when wrapped, so forget about that.

A quick comment should do the job re: understanding.

--
Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 15+ 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)
                   ` (2 preceding siblings ...)
  2026-10-14  6:02 ` Baolin Wang
@ 2026-10-14 16:30 ` Lance Yang
  3 siblings, 0 replies; 15+ messages in thread
From: Lance Yang @ 2026-10-14 16:30 UTC (permalink / raw)
  To: david
  Cc: akpm, ljs, liam, vbabka, rppt, surenb, mhocko, huang.ying.caritas,
	baolin.wang, baohua, ryan.roberts, hanchuanhua, kas, kylebot,
	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;

That -1/+1 threw me for a sec ... Neat little trick!

>+	const unsigned long offset = idx << PAGE_SHIFT;
>+
>+	/* Avoid any address wraparounds. */
>+	*start = addr - min(addr - lower, offset);
>+	*end = addr + min(upper - addr, 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 */
>
>---

Nothing jumped out at me, feel free to add:

Reviewed-by: Lance Yang <lance.yang@linux.dev>


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

end of thread, other threads:[~2026-10-14 16:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-14  8:03     ` David Hildenbrand (Arm)
2026-10-14  8:04   ` David Hildenbrand (Arm)
2026-10-13 14:11 ` Lorenzo Stoakes (ARM)
2026-10-14  8:12   ` David Hildenbrand (Arm)
2026-10-14 13:27     ` Lorenzo Stoakes (ARM)
2026-10-14 13:45       ` David Hildenbrand (Arm)
2026-10-14 13:49         ` David Hildenbrand (Arm)
2026-10-14 14:57           ` Lorenzo Stoakes (ARM)
2026-10-14 14:56         ` Lorenzo Stoakes (ARM)
2026-10-14  6:02 ` Baolin Wang
2026-10-14  8:02   ` David Hildenbrand (Arm)
2026-10-14 16:30 ` Lance Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.