[PATCH] mm: fix address wraparound when scanning large folio PTEs

[PATCH] mm: fix address wraparound when scanning large folio PTEs

[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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[Kiryl Shutsemau]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[Barry Song]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[David Hildenbrand (Arm)]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[David Hildenbrand (Arm)]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[Lorenzo Stoakes (ARM)]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[David Hildenbrand (Arm)]

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

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[Baolin Wang]

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>

Re: [PATCH] mm: fix address wraparound when scanning large folio PTEs

[David Hildenbrand (Arm)]

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