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@xxxxxxxxxx> 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@xxxxxxxxxx>
>>> Fixes: d2136d749d76 ("mm: support multi-size THP numa balancing")
>>> Fixes: 508758960b8d ("mm: swap: entirely map large folios found in swapcache")
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>>> ---
>>>  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