[PATCH v3] memalign: reinstate alignment overflow check (CVE-2026-0861)

[Siddhesh Poyarekar]

On 2026-01-15 10:31, Wilco Dijkstra wrote:
> Hi Siddhesh,
> 
> You really opened a can of worms here. All of the arena_get and arena_get_retry
> calls can overflow since they are passed the *original* user size. So checking for

OK, lets shut that can a bit then ;)  My intent here is to constrain the 
fix to specifically CVE-2026-0861 and I've added both checks because 
they existed before and still make sense.

> overflow in one case isn't going to help at all. The arena code will add more to it,
> again without any checks:
> 
>    h = new_heap (size + (sizeof (*h) + sizeof (*a) + MALLOC_ALIGNMENT),

Having a heap larger than PTRDIFF_MAX (if the system is actually able to 
allocate it of course) is OK from the user perspective since the user is 
only getting a chunk that is within PTRDIFF_MAX bounds.  That 
computation does not overflow size_t as long as size is checked to be 
within PTRDIFF_MAX bounds.

> Size is the value passed to arena_get(_retry), but that is typically the user
> size, eg:
> 
> static void * __attribute_noinline__
> __libc_malloc2 (size_t bytes)
> {
>    mstate ar_ptr;
>    void *victim;

This needs an early bounds check fail then.

> 
>    if (SINGLE_THREAD_P)
>      {
>        victim = tag_new_usable (_int_malloc (&main_arena, bytes));
>        assert (!victim || chunk_is_mmapped (mem2chunk (victim)) ||
>                &main_arena == arena_for_chunk (mem2chunk (victim)));
>        return victim;
>      }
> 
>    arena_get (ar_ptr, bytes);
> 
>    victim = _int_malloc (ar_ptr, bytes);
> 
> Same for calloc2.

Likewise.

> 
> -  arena_get (ar_ptr, bytes + alignment + MINSIZE);
> +  ptrdiff_t total;
> +  /* ALIGNMENT is a power of two, so adding MINSIZE won't overflow it.  */
> +  if (__glibc_unlikely (__builtin_add_overflow (bytes,
> +						alignment + MINSIZE,
> +						&total)))
> +    {
> +      __set_errno (ENOMEM);
> +      return NULL;
> +    }
> +
> +  arena_get (ar_ptr, total);
> 
> This particular unsigned->signed add overflow is quite complex and adds a
> lot of extra code. All unnecessary complexity since arena_get doesn't even use
> the value in almost all cases - it's only used in the rare case when it creates a
> new arena. And it's not clear there is any advantage in that case, let alone
> trying to compute an accurate value of the total size...
> 
> So I don't see any point in passing anything to arena_get here - just use
> arena_get (ar_ptr, 0) everywhere.

Or of course, use arena_get (ar_ptr, 0) and avoid the problem 
altogether, which has the problem of the arena maybe being insufficient. 
  Anyway, I'm shutting this can by dropping this hunk, I'll limit the 
change to just _int_memalign; I feel like Florian had seen this coming 
when he suggested that ;)

> +  ptrdiff_t total;
> +  /* ALIGNMENT is a power of two, so adding MINSIZE won't overflow it.  */
> +  if (__glibc_unlikely (__builtin_add_overflow (nb,
> +						alignment + MINSIZE,
> +						&total)))
> +    {
> +      __set_errno (ENOMEM);
> +      return NULL;
> +    }
> 
> Same here, a lot of code to write: if (alignment > PTRDIFF_MAX) when then trivially
> merges with the earlier if statement.

It doesn't though, since bytes <= PTRDIFF_MAX does not imply nb <= 
PTRDIFF_MAX.  Also, even with nb == alignment == PTRDIFF_MAX, the checks 
will pass, but nb + alignment + MINSIZE will overflow.

Sid