Re: [PATCH v3 03/10] mm/vmalloc: clamp an unaligned range in vunmap_pte_range()

[Dev Jain <dev.jain@arm.com>]

On 30/10/26 2:00 pm, Hao Ge wrote:
> Hi Dev
> 
> Thanks for your review.
> 
> On 2026/10/30 15:14, Dev Jain wrote:
>>
>>
>> On 26/10/26 1:33 pm, Hao Ge wrote:
>>> commit a739e6b557af ("mm: vmalloc: WARN_ON if mapping size is not
>>> PAGE_SIZE aligned") made vmap_pte_range() warn and bail out when the
>>> size is not a multiple of PAGE_SIZE. That loop steps in page
>>
>> Why are we checking the alignment so late into vmap_pte_range? Why not
>> simply do
>>
>> if (!IS_ALIGNED(addr, PAGE_SIZE) || !IS_ALIGNED(end, PAGE_SIZE))
>> 	return; (or return -EINVAL);
>>
>> at the start of vmap_range_noflush and vunmap_range_noflush?
> 
> I think adding that check at the entry of vunmap_range_noflush()
> goes against its design intent. Its job is to tear down the mappings
> of the given VA range, so even when the caller passes a size that is
> not page-aligned, we still have to unmap whatever is mapped within
> that range.
> 
> So the patch clamps end down to the last full page and keeps
> clearing. A buggy caller still gets its mapping torn down as far as
> it goes, and nothing is left behind. The check here is just for
> protection. 
> 
> This is just my preliminary understanding; I may be mistaken.

Hmm I get you, but this is very ugly. We are trying to tolerate
a blatant bug in the caller with not-so-nice code. But see below.
> 
> Thanks
> Best Regards
> Hao
> 
> 
> But that's just my understanding, happy to be corrected.
> 
> 
>>
>>
>>> multiples and stops only at addr == end; with an unaligned size addr
>>> never lands on end, so the loop cannot terminate and walks off the
>>> end of the page table.
>>>
>>> vunmap_pte_range() has the same loop without any guard, so an
>>> unaligned range would clear PTEs past end without ever stopping,
>>> walking off the end of the page table.
>>>
>>> Add the check there too. The range is clamped down to the last full
>>> page rather than dropped, and the loop is skipped when nothing is
>>> left, so a buggy caller still gets its PTEs cleared as far as they
>>> go.
>>>
>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>>> ---
>>>  mm/vmalloc.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>>> index 44e0eb32708f..7131f60d1a8e 100644
>>> --- a/mm/vmalloc.c
>>> +++ b/mm/vmalloc.c
>>> @@ -393,6 +393,11 @@ static void vunmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>>>  	pte_t ptent;
>>>  	unsigned long size = PAGE_SIZE;
>>>  
>>> +	if (WARN_ON_ONCE(!PAGE_ALIGNED(end - addr)))
>>> +		end = addr + PAGE_ALIGN_DOWN(end - addr);

For simplicity assume PAGE_SIZE is 4, start is 3, end is 10.
With your code we will get

addr = 3, end = 3 + ALIGN_DOWN(10 - 3) = 3 + 4 = 7.
So the range becomes [3, 7) clearing only two ptes, the one covering
[0, 3], the second covering [4, 7]. I feel we should rather make
the range from [3, 10] to [0, 11], clearing three ptes.

So, at the start of vunmap_range_noflush we could basically
WARN_ON on non-alignment, and then clamp the start down, and clamp
the end up to PAGE_SIZE.

And with this also throw in a patch to bring the -EINVAL bailout
in vmap_pte_range to the top, in vmap_range_noflush and check
both start and end alignment.

>>> +	if (end == addr)
>>> +		return;
>>> +
>>>  	pte = pte_offset_kernel(pmd, addr);
>>>  	lazy_mmu_mode_enable();
>>>  
>>