mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
@ 2026-10-22 23:35 Shubhang Kaushik (Ampere)
  2026-10-23  3:19 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Shubhang Kaushik (Ampere) @ 2026-10-22 23:35 UTC (permalink / raw)
  To: Andrew Morton, Uladzislau Rezki, Nicholas Piggin,
	Shubhang Kaushik, Christoph Lameter (Ampere)
  Cc: linux-mm, linux-kernel, Shubhang Kaushik (Ampere)

vm_map_ram() converts its unsigned int page count to an unsigned long
byte size without checking for overflow.

On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
and vmap_pages_range(), so it can allocate and map only the wrapped
number of pages while returning success for the original larger request.

For example, with 4 KiB pages, a count of 1048577 produces a
PAGE_SIZE byte size after wrapping, allowing a one-page mapping.

Reject page counts whose byte size cannot be represented.

Fixes: db64fe02258f ("mm: rewrite vmap layer")
Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
---
 mm/vmalloc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index bea9f76ed7e74247d4ce919250c2f02dd5c36483..e98d3aa551fe205ebd9826daad9ad6bd578b91ea 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3083,10 +3083,15 @@ EXPORT_SYMBOL(vm_unmap_ram);
  */
 void *vm_map_ram(struct page **pages, unsigned int count, int node)
 {
-	unsigned long size = (unsigned long)count << PAGE_SHIFT;
+	unsigned long size;
 	unsigned long addr;
 	void *mem;
 
+	if (count > ULONG_MAX >> PAGE_SHIFT)
+		return NULL;
+
+	size = (unsigned long)count << PAGE_SHIFT;
+
 	if (likely(count <= VMAP_MAX_ALLOC)) {
 		mem = vb_alloc(size, GFP_KERNEL);
 		if (IS_ERR(mem))

---
base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
change-id: 20261022-vmalloc-reject-overflow-e532c556f3fa

Best regards,
-- 
Shubhang Kaushik (Ampere) <sh@gentwo.org>


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

* Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
  2026-10-22 23:35 [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts Shubhang Kaushik (Ampere)
@ 2026-10-23  3:19 ` Andrew Morton
  2026-10-23  6:12   ` Shubhang
  2026-10-23  6:54 ` Dev Jain
  2026-10-23 11:22 ` Anshuman Khandual
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2026-10-23  3:19 UTC (permalink / raw)
  To: Shubhang Kaushik (Ampere)
  Cc: Uladzislau Rezki, Nicholas Piggin, Shubhang Kaushik,
	Christoph Lameter (Ampere),
	linux-mm, linux-kernel

On Thu, 22 Oct 2026 16:35:20 -0700 "Shubhang Kaushik (Ampere)" <sh@gentwo.org> wrote:

> vm_map_ram() converts its unsigned int page count to an unsigned long
> byte size without checking for overflow.
> 
> On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
> the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
> and vmap_pages_range(), so it can allocate and map only the wrapped
> number of pages while returning success for the original larger request.
> 
> For example, with 4 KiB pages, a count of 1048577 produces a
> PAGE_SIZE byte size after wrapping, allowing a one-page mapping.
> 
> Reject page counts whose byte size cannot be represented.

Thanks,

When fixing a bug, please always fully describe the worst-case
userspace-visible runtime effects of that bug.

AI tells me:

: On 32-bit kernels an overflowing count can currently cause vm_map_ram()
: to return a non-NULL pointer to a mapping smaller than requested.  The
: patch makes such requests fail before allocating anything.
:
: However, I haven't identified a kernel caller through which
: userspace can trigger this condition. The callers I've examined
: appear to constrain the page count appropriately.
:
: So I'd describe this as hardening against invalid kernel API
: arguments rather than fixing a demonstrated userspace-visible
: failure.


Also,

: On 64-bit kernels count is an unsigned int, so the comparison against
: ULONG_MAX >> PAGE_SHIFT is always false.  Clang W=1 can warn about
: this, potentially breaking WERROR builds.
:
: Could we instead use:
:
:	if (check_mul_overflow((unsigned long)count, PAGE_SIZE, &size))
:		return NULL;



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

* Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
  2026-10-23  3:19 ` Andrew Morton
@ 2026-10-23  6:12   ` Shubhang
  0 siblings, 0 replies; 6+ messages in thread
From: Shubhang @ 2026-10-23  6:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Uladzislau Rezki, Nicholas Piggin, Shubhang Kaushik,
	Christoph Lameter (Ampere),
	linux-mm, linux-kernel

Hi Andrew,

On Thu, 22 Oct 2026, Andrew Morton wrote:

> On Thu, 22 Oct 2026 16:35:20 -0700 "Shubhang Kaushik (Ampere)" <sh@gentwo.org> wrote:
>
>> vm_map_ram() converts its unsigned int page count to an unsigned long
>> byte size without checking for overflow.
>>
>> On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
>> the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
>> and vmap_pages_range(), so it can allocate and map only the wrapped
>> number of pages while returning success for the original larger request.
>>
>> For example, with 4 KiB pages, a count of 1048577 produces a
>> PAGE_SIZE byte size after wrapping, allowing a one-page mapping.
>>
>> Reject page counts whose byte size cannot be represented.
>
> Thanks,
>
> When fixing a bug, please always fully describe the worst-case
> userspace-visible runtime effects of that bug.
>
> AI tells me:
>
> : On 32-bit kernels an overflowing count can currently cause vm_map_ram()
> : to return a non-NULL pointer to a mapping smaller than requested.  The
> : patch makes such requests fail before allocating anything.
> :
> : However, I haven't identified a kernel caller through which
> : userspace can trigger this condition. The callers I've examined
> : appear to constrain the page count appropriately.
> :
> : So I'd describe this as hardening against invalid kernel API
> : arguments rather than fixing a demonstrated userspace-visible
> : failure.
>
>
> Also,
>
> : On 64-bit kernels count is an unsigned int, so the comparison against
> : ULONG_MAX >> PAGE_SHIFT is always false.  Clang W=1 can warn about
> : this, potentially breaking WERROR builds.
> :
> : Could we instead use:
> :
> :	if (check_mul_overflow((unsigned long)count, PAGE_SIZE, &size))
> :		return NULL;
>

Thanks, agreed. I'll respin this using check_mul_overflow().

I'll also expand the changelog to describe the failure mode. On a 32-bit
kernel, converting a requested page count to a byte size can overflow. 
vm_map_ram() can then return success after mapping fewer pages than 
requested. If a caller later accesses the returned address as if all 
requested pages were mapped, an access to one of the omitted pages can 
fault. If another vmalloc mapping occupies that virtual address, the 
access can instead reach unrelated kernel memory. Userspace can observe 
this as a failed operation or a kernel oops/panic; in the worst case, the 
erroneous access can corrupt kernel memory.

I found that udmabuf can pass a userspace-controlled page count to
vm_map_ram() when a DMA-BUF importer invokes its vmap operation. 
However, for the same oversized count, udmabuf also truncates both the 
advertised DMA-BUF size and the byte length used for its SG table.
That said, I have not found a current in-tree importer that accesses the 
returned mapping as if it covered all of the originally requested pages.

Regards,
Shubhang Kaushik

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

* Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
  2026-10-22 23:35 [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts Shubhang Kaushik (Ampere)
  2026-10-23  3:19 ` Andrew Morton
@ 2026-10-23  6:54 ` Dev Jain
  2026-10-23  7:37   ` Shubhang
  2026-10-23 11:22 ` Anshuman Khandual
  2 siblings, 1 reply; 6+ messages in thread
From: Dev Jain @ 2026-10-23  6:54 UTC (permalink / raw)
  To: Shubhang Kaushik (Ampere),
	Andrew Morton, Uladzislau Rezki, Nicholas Piggin,
	Shubhang Kaushik, Christoph Lameter (Ampere)
  Cc: linux-mm, linux-kernel



On 23/10/26 5:05 am, Shubhang Kaushik (Ampere) wrote:
> vm_map_ram() converts its unsigned int page count to an unsigned long
> byte size without checking for overflow.
> 
> On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
> the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
> and vmap_pages_range(), so it can allocate and map only the wrapped
> number of pages while returning success for the original larger request.

I would expect the caller to take care of this?

In vmap(), the check is count > totalram_pages(), is this check more
appropriate to be used in vm_map_ram too?

> 
> For example, with 4 KiB pages, a count of 1048577 produces a
> PAGE_SIZE byte size after wrapping, allowing a one-page mapping.
> 
> Reject page counts whose byte size cannot be represented.
> 
> Fixes: db64fe02258f ("mm: rewrite vmap layer")
> Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
> ---
>  mm/vmalloc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index bea9f76ed7e74247d4ce919250c2f02dd5c36483..e98d3aa551fe205ebd9826daad9ad6bd578b91ea 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3083,10 +3083,15 @@ EXPORT_SYMBOL(vm_unmap_ram);
>   */
>  void *vm_map_ram(struct page **pages, unsigned int count, int node)
>  {
> -	unsigned long size = (unsigned long)count << PAGE_SHIFT;
> +	unsigned long size;
>  	unsigned long addr;
>  	void *mem;
>  
> +	if (count > ULONG_MAX >> PAGE_SHIFT)
> +		return NULL;
> +
> +	size = (unsigned long)count << PAGE_SHIFT;
> +
>  	if (likely(count <= VMAP_MAX_ALLOC)) {
>  		mem = vb_alloc(size, GFP_KERNEL);
>  		if (IS_ERR(mem))
> 
> ---
> base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
> change-id: 20261022-vmalloc-reject-overflow-e532c556f3fa
> 
> Best regards,


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

* Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
  2026-10-23  6:54 ` Dev Jain
@ 2026-10-23  7:37   ` Shubhang
  0 siblings, 0 replies; 6+ messages in thread
From: Shubhang @ 2026-10-23  7:37 UTC (permalink / raw)
  To: Dev Jain
  Cc: Andrew Morton, Uladzislau Rezki, Nicholas Piggin,
	Shubhang Kaushik, Christoph Lameter (Ampere),
	linux-mm, linux-kernel

Hi Dev,

On Fri, 23 Oct 2026, Dev Jain wrote:

>
>
> On 23/10/26 5:05 am, Shubhang Kaushik (Ampere) wrote:
>> vm_map_ram() converts its unsigned int page count to an unsigned long
>> byte size without checking for overflow.
>>
>> On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
>> the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
>> and vmap_pages_range(), so it can allocate and map only the wrapped
>> number of pages while returning success for the original larger request.
>
> I would expect the caller to take care of this?
>
> In vmap(), the check is count > totalram_pages(), is this check more
> appropriate to be used in vm_map_ram too?
>

Thanks for the review. I have addressed the conversion issue in v2
with check_mul_overflow(), as suggested by Andrew:

   https://lore.kernel.org/lkml/20261022-vmalloc-reject-overflow-v2-1-0db7a20c53f0@gentwo.org/

I do not think the totalram_pages() check in vmap() is an appropriate
replacement for the overflow check in vm_map_ram(). It is a separate
sanity check on the requested page count; it does not ensure that
count * PAGE_SIZE is representable in an unsigned long.

For example, on a 32-bit PAE kernel, totalram_pages() can exceed
ULONG_MAX >> PAGE_SHIFT, so a count can pass that check while its byte
size still overflows.

vm_map_ram() is also a separate API and performs this conversion itself,
so the check_mul_overflow() validation should remain.

Regards,
Shubhang Kaushik

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

* Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts
  2026-10-22 23:35 [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts Shubhang Kaushik (Ampere)
  2026-10-23  3:19 ` Andrew Morton
  2026-10-23  6:54 ` Dev Jain
@ 2026-10-23 11:22 ` Anshuman Khandual
  2 siblings, 0 replies; 6+ messages in thread
From: Anshuman Khandual @ 2026-10-23 11:22 UTC (permalink / raw)
  To: Shubhang Kaushik (Ampere)
  Cc: Andrew Morton, Uladzislau Rezki, Nicholas Piggin,
	Shubhang Kaushik, Christoph Lameter (Ampere),
	linux-mm, linux-kernel

On Thu, Oct 22, 2026 at 04:35:20PM -0700, Shubhang Kaushik (Ampere) wrote:
> vm_map_ram() converts its unsigned int page count to an unsigned long
> byte size without checking for overflow.
> 
> On 32-bit kernels, a count greater than ULONG_MAX >> PAGE_SHIFT wraps
> the byte size.  vm_map_ram() passes that size to both alloc_vmap_area()
> and vmap_pages_range(), so it can allocate and map only the wrapped
> number of pages while returning success for the original larger request.
> 
> For example, with 4 KiB pages, a count of 1048577 produces a
> PAGE_SIZE byte size after wrapping, allowing a one-page mapping.
> 
> Reject page counts whose byte size cannot be represented.
> 
> Fixes: db64fe02258f ("mm: rewrite vmap layer")
> Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
> ---
>  mm/vmalloc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index bea9f76ed7e74247d4ce919250c2f02dd5c36483..e98d3aa551fe205ebd9826daad9ad6bd578b91ea 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3083,10 +3083,15 @@ EXPORT_SYMBOL(vm_unmap_ram);
>   */
>  void *vm_map_ram(struct page **pages, unsigned int count, int node)
>  {
> -	unsigned long size = (unsigned long)count << PAGE_SHIFT;
> +	unsigned long size;
>  	unsigned long addr;
>  	void *mem;
>  
> +	if (count > ULONG_MAX >> PAGE_SHIFT)
> +		return NULL;

Could vm_map_ram() ever be called with such a large number as count ?

> +
> +	size = (unsigned long)count << PAGE_SHIFT;
> +
>  	if (likely(count <= VMAP_MAX_ALLOC)) {
>  		mem = vb_alloc(size, GFP_KERNEL);
>  		if (IS_ERR(mem))
> 
> ---
> base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
> change-id: 20261022-vmalloc-reject-overflow-e532c556f3fa
> 
> Best regards,
> -- 
> Shubhang Kaushik (Ampere) <sh@gentwo.org>
> 

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

end of thread, other threads:[~2026-10-23 11:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 23:35 [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts Shubhang Kaushik (Ampere)
2026-10-23  3:19 ` Andrew Morton
2026-10-23  6:12   ` Shubhang
2026-10-23  6:54 ` Dev Jain
2026-10-23  7:37   ` Shubhang
2026-10-23 11:22 ` Anshuman Khandual

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®