Re: [PATCH 1/2] tools/nolibc: check for overflow in malloc()

[Thomas Weißschuh]

On 2026-09-26 08:43:31-0500, Danish Khateeb wrote:
> malloc() adds the size of its header to the requested size and rounds
> the sum up to a multiple of 4096, without checking either step for
> overflow. For a size within the header size of SIZE_MAX, the sum wraps
> around to a few bytes, and malloc() returns a single page instead of
> NULL. For a slightly smaller size, the rounding wraps around to 0, and
> malloc() fails with EINVAL from mmap() instead of ENOMEM.
> 
> calloc() checks its multiplication for overflow, but then passes the
> product to malloc(), so calloc(SIZE_MAX, 1) returns a single page too.
> So does realloc(ptr, SIZE_MAX).
> 
> Such a size is usually a bug in the caller, for instance a length of -1
> used as a size_t, and returning a page instead of NULL can turn it into
> a heap overflow.
> 
> Fail with ENOMEM when adding the header or rounding up overflows, as
> glibc does for sizes this large.
> 
> Fixes: 0e0ff638400b ("tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`")
> Assisted-by: LLM
> Signed-off-by: Danish Khateeb <danishkhateeb03@xxxxxxxxx>
> ---
>  tools/include/nolibc/stdlib.h | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h
> index 6c3c620a04cf..ea315e60cdeb 100644
> --- a/tools/include/nolibc/stdlib.h
> +++ b/tools/include/nolibc/stdlib.h
> @@ -130,9 +130,15 @@ void *malloc(size_t len)
>  {
>  	struct nolibc_heap *heap;
>  
> -	/* Always allocate memory with size multiple of 4096. */
> -	len  = sizeof(*heap) + len;
> -	len  = (len + 4095UL) & -4096UL;
> +	/*
> +	 * Always allocate memory with size multiple of 4096, and reject sizes
> +	 * which would wrap around once the header is added and rounded up.
> +	 */

The rounding to a multiple to 4096 is probably meant to align the size
to a page boundary. But this is insufficient on systems where the page
size is not 4k. The kernel doesn't actually seem to care.
Let's drop the rounding instead, and then add the overflow check on top.

Also the unittest doesn't seem to be worth it.

> +	if (__builtin_expect(__builtin_add_overflow(len, sizeof(*heap) + 4095UL, &len), 0)) {
> +		SET_ERRNO(ENOMEM);
> +		return NULL;
> +	}
> +	len &= -4096UL;
>  	heap = mmap(NULL, len, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE,
>  		    -1, 0);
>  	if (__builtin_expect(heap == MAP_FAILED, 0))
> -- 
> 2.55.0
>