mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE aligned

[PATCH] mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE aligned

Posted by Yadong Qi 2 months, 1 week ago

In mm/vmalloc.c, the function vmap_pte_range() assumes that the
mapping size is aligned to PAGE_SIZE. If this assumption is
violated, the loop will become infinite because the termination
condition (`addr != end`) will never be met. This can lead to
overwriting other VA ranges and/or random pages physically follow
the page table.

It's the caller's responsibility to ensure that the mapping size
is aligned to PAGE_SIZE. However, the memory corruption is hard
to root cause. To identify the programming error in the caller
easier, check whether the mapping size is PAGE_SIZE aligned with
BUG_ON().

Signed-off-by: Yadong Qi <yadong.qi@linux.alibaba.com>
Reviewed-by: Huang Ying <ying.huang@linux.alibaba.com>
---
 mm/vmalloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 5edd536ba9d2..b54d3ee6b202 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -100,6 +100,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 	struct page *page;
 	unsigned long size = PAGE_SIZE;
 
+	BUG_ON(!PAGE_ALIGNED(end - addr));
+
 	pfn = phys_addr >> PAGE_SHIFT;
 	pte = pte_alloc_kernel_track(pmd, addr, mask);
 	if (!pte)
-- 
2.43.5

Re: [PATCH] mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE aligned

Posted by Andrew Morton 2 months, 1 week ago

On Thu,  9 Oct 2025 11:59:43 +0800 Yadong Qi <yadong.qi@linux.alibaba.com> wrote:

> In mm/vmalloc.c, the function vmap_pte_range() assumes that the
> mapping size is aligned to PAGE_SIZE. If this assumption is
> violated, the loop will become infinite because the termination
> condition (`addr != end`) will never be met. This can lead to
> overwriting other VA ranges and/or random pages physically follow
> the page table.
> 
> It's the caller's responsibility to ensure that the mapping size
> is aligned to PAGE_SIZE. However, the memory corruption is hard
> to root cause. To identify the programming error in the caller
> easier, check whether the mapping size is PAGE_SIZE aligned with
> BUG_ON().
> 
> ..
>
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -100,6 +100,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  	struct page *page;
>  	unsigned long size = PAGE_SIZE;
>  
> +	BUG_ON(!PAGE_ALIGNED(end - addr));
> +
>  	pfn = phys_addr >> PAGE_SHIFT;
>  	pte = pte_alloc_kernel_track(pmd, addr, mask);
>  	if (!pte)

We try to avoid adding BUG()s - deliberately crashing the kernel is
pretty cruel to the user.  It's far better to WARN and to continue in
some fashion so the user can at least gather logs, etc.

How about

	if (WARN_ON(!PAGE_ALIGNED(end - addr)))
		return -ENOMEM;

?

(Or VM_WARN_ON)

RE: [PATCH] mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE aligned

Posted by yadong.qi@linux.alibaba.com 2 months, 1 week ago

> -----Original Message-----
> From: Andrew Morton <akpm@linux-foundation.org>
> Sent: 2025年10月9日 12:17
> To: Yadong Qi <yadong.qi@linux.alibaba.com>
> Cc: urezki@gmail.com; linux-mm@kvack.org; linux-kernel@vger.kernel.org;
> ying.huang@linux.alibaba.com
> Subject: Re: [PATCH] mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE
> aligned
> 
> On Thu,  9 Oct 2025 11:59:43 +0800 Yadong Qi <yadong.qi@linux.alibaba.com>
> wrote:
> 
> > In mm/vmalloc.c, the function vmap_pte_range() assumes that the
> > mapping size is aligned to PAGE_SIZE. If this assumption is
> > violated, the loop will become infinite because the termination
> > condition (`addr != end`) will never be met. This can lead to
> > overwriting other VA ranges and/or random pages physically follow
> > the page table.
> >
> > It's the caller's responsibility to ensure that the mapping size
> > is aligned to PAGE_SIZE. However, the memory corruption is hard
> > to root cause. To identify the programming error in the caller
> > easier, check whether the mapping size is PAGE_SIZE aligned with
> > BUG_ON().
> >
> > ..
> >
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -100,6 +100,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long
> addr, unsigned long end,
> >  	struct page *page;
> >  	unsigned long size = PAGE_SIZE;
> >
> > +	BUG_ON(!PAGE_ALIGNED(end - addr));
> > +
> >  	pfn = phys_addr >> PAGE_SHIFT;
> >  	pte = pte_alloc_kernel_track(pmd, addr, mask);
> >  	if (!pte)
> 
> We try to avoid adding BUG()s - deliberately crashing the kernel is
> pretty cruel to the user.  It's far better to WARN and to continue in
> some fashion so the user can at least gather logs, etc.
> 
> How about
> 
> 	if (WARN_ON(!PAGE_ALIGNED(end - addr)))
> 		return -ENOMEM;
> 
> ?
> 
> (Or VM_WARN_ON)

Sure, I will send a new patch with WARN_ON.

Best Regard
Yadong

Re: [PATCH] mm: vmalloc: BUG_ON if mapping size is not PAGE_SIZE aligned

Posted by Lance Yang 2 months, 1 week ago

>> In mm/vmalloc.c, the function vmap_pte_range() assumes that the
>> mapping size is aligned to PAGE_SIZE. If this assumption is
>> violated, the loop will become infinite because the termination
>> condition (`addr != end`) will never be met. This can lead to
>> overwriting other VA ranges and/or random pages physically follow
>> the page table.
>> 
>> It's the caller's responsibility to ensure that the mapping size
>> is aligned to PAGE_SIZE. However, the memory corruption is hard
>> to root cause. To identify the programming error in the caller
>> easier, check whether the mapping size is PAGE_SIZE aligned with
>> BUG_ON().
>> 
>> ..
>>
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -100,6 +100,8 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>>  	struct page *page;
>>  	unsigned long size = PAGE_SIZE;
>>  
>> +	BUG_ON(!PAGE_ALIGNED(end - addr));
>> +
>>  	pfn = phys_addr >> PAGE_SHIFT;
>>  	pte = pte_alloc_kernel_track(pmd, addr, mask);
>>  	if (!pte)
>
>We try to avoid adding BUG()s - deliberately crashing the kernel is
>pretty cruel to the user.  It's far better to WARN and to continue in
>some fashion so the user can at least gather logs, etc.
>
>How about
>
>	if (WARN_ON(!PAGE_ALIGNED(end - addr)))
>		return -ENOMEM;
>
>?
>
>(Or VM_WARN_ON)

I agree with Andrew. Using WARN_ON/VM_WARN_ON and returning an error is
the way to go.

AFAIK, we are moving away from BUG_ON() in MM whenever an error can be
handled gracefully.

Cheers,
Lance