Subject: Re: [PATCH] evaluate the second argument of ALLOC_GROW only once

From: Jeff King <[email-masked]>
Date: Mon, 18 May 2026 20:41:43 -0400

On Sat, May 16, 2026 at 08:55:54AM +0200, Johannes Sixt wrote:

> > Hmm, playing with it and looking a little closer, I think we don't end
> > up overflowing the buffer because you use the size_t for
> > REALLOC_ARRAY(). So the result is big, but then "alloc" is truncated.
> 
> Protect against double-evaluation of "alloc", too, using
> 
> 	size_t *palloc = &(alloc);
> 
> and use *palloc in the two places, then all callers are forced to work
> with a size_t as third argument. Don't know what the damage would be,
> though.

I think it would be nice if all ALLOC_GROW() callers used a size_t, and
then we checked the size_t computation for overflow. But from a rough
guess (taking your suggestion and trying to compile) we'd need to adjust
~200 callers.

And it's not just a straight conversion:

  1. Sometimes the ability to represent a negative value is important,
     and each site has to be audited. If we could agree on a "as big as
     size_t but signed" type, that might help.

  2. Changing the alloc variable type without the matching "nr" can
     actually make things worse. We tend to catch overflow-by-1 for
     signed types incidentally because it results in a stupidly large
     allocation request. But if made our allocations correct, then we
     might overflow on "nr" and start writing to some huge negative
     offset before the array.

So I think it would be a fair bit of work, though I would feel better
about the resulting state.

-Peff