Re: [PATCH] mm/vmalloc: reject overflowing vm_map_ram page counts

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

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,