[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <hn3ny53myumx64caoaiphbco6wdxfuvxq4hyawg2ezf77hkwxf@5hyw7t4gs74f>
Date: Wed, 25 Jun 2025 01:01:01 +0200
From: Alejandro Colomar <alx@...nel.org>
To: Eric Blake <eblake@...hat.com>
Cc: libc-alpha@...rceware.org, bug-gnulib@....org, musl@...ts.openwall.com,
наб <nabijaczleweli@...ijaczleweli.xyz>, Douglas McIlroy <douglas.mcilroy@...tmouth.edu>,
Paul Eggert <eggert@...ucla.edu>, Robert Seacord <rcseacord@...il.com>,
Elliott Hughes <enh@...gle.com>, Bruno Haible <bruno@...sp.org>,
JeanHeyd Meneide <phdofthehouse@...il.com>, Rich Felker <dalias@...c.org>,
Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>, Joseph Myers <josmyers@...hat.com>,
Florian Weimer <fweimer@...hat.com>, Laurent Bercot <ska-dietlibc@...rnet.org>,
Andreas Schwab <schwab@...e.de>, Vincent Lefevre <vincent@...c17.net>,
Mark Harris <mark.hsj@...il.com>, Collin Funk <collin.funk1@...il.com>,
Wilco Dijkstra <Wilco.Dijkstra@....com>, DJ Delorie <dj@...hat.com>,
Cristian Rodríguez <cristian@...riguez.im>, Siddhesh Poyarekar <siddhesh@...plt.org>,
Sam James <sam@...too.org>, Mark Wielaard <mark@...mp.org>,
"Maciej W. Rozycki" <macro@...hat.com>, Martin Uecker <ma.uecker@...il.com>,
Christopher Bazley <chris.bazley.wg14@...il.com>, eskil@...ession.se,
Daniel Krügler <daniel.kruegler@...glemail.com>, Kees Cook <keescook@...omium.org>,
Valdis Klētnieks <valdis.kletnieks@...edu>
Subject: Re: alx-0029r3 - Restore the traditional realloc(3) specification
Hi Eric,
On Tue, Jun 24, 2025 at 04:18:40PM -0500, Eric Blake wrote:
> On Tue, Jun 24, 2025 at 07:01:50AM +0200, Alejandro Colomar wrote:
[...]
> Some feedback to consider:
> ...
>
> > Description
[...]
> > [3] <https://gbhackers.com/whatsapp-double-free-vulnerability/>
> >
> > However, this doesn't need to be like that. The traditional
> > implementation of realloc(3), present in Unix V7, inherited by
> > the BSDs, and currently available in range of systems, including
> > musl libc, doesn't have any issues.
>
> It may be worth specifically mentioning that glibc 2.1 and earlier
> also had that behavior, even though it was an independent
> implementation not derived from Unix V7; and that the _only_ reason
> glibc 2.2 and later changed in 2000 to just freeing p instead of
> returning a result like malloc(0) was because someone argued that the
> C89 wording required that change, despite the new wording in C99 in
> discussion under the time that was trying to remove the warts in the
> C89 definition.
Certainly; I'll add that.
[...]
> > Most code is written in this way, even if run on platforms
> > returning a null pointer. This is because most programmers are
> > just unaware of this problem.
>
> It may also be worth pointing out that any time code behaves one way
> for 3, 2, 1, and then suddenly changes behavior at 0, it is much
> harder to code the use of that interface correctly; when compared to
> an interface where each successive call is merely one byte less in
> effect than the previous-larger call.
Yup.
[...]
> > Prior art
[...]
> > Unix V7
> > The proposed behavior is the one endorsed by Doug McIlroy, the
> > author of the original implementation of realloc(3) in Unix V7,
> > and also present in the BSDs.
>
> Would calling out glibc 2.1 as prior art help?
Yup.
> > Design decisions
> > This change needs three changes, which can be applied all at
> > once, or in separate steps.
>
> You document "three" here...
Yeah, I forgot to s/three/two/ when I removed the requirement to set
ENOMEM.
[...]
> ...and no mention of the third step. You'll want to clean that up.
Yup.
[...]
> > Caveats
> > n?n:1
> > Code written today should be careful, in case it can run on
> > older systems that are not fixed to comply with this stricter
> > specification. Thus, code written today should call realloc(3)
> > similar to this:
> >
> > realloc(p, n?n:1);
> >
> > When all existing implementations are fixed to comply with this
> > stricter specification, that workaround can be removed.
> >
> > ENOMEM
> > Existing implementations that set errno to ENOMEM must continue
> > doing so when the input pointer is not freed. If they didn't,
> > code that is currently portable to all POSIX systems
> >
> > errno = 0;
> > new = realloc(old, size);
> > if (new == NULL) {
> > if (errno == ENOMEM)
> > free(old);
> > goto fail;
> > }
> > ...
> > free(new);
> >
> > would leak on error.
>
> Would it also be worth mentioning (either here or as a footnote to be
> added in the standard) that while atypical, realloc() is allowed to
> fail with ENOMEM even when the new size is smaller than the previous
> size of the pointer? This might be seen as a non-intuitive result,
> but as Rich Felker pointed out, there ARE implementations of malloc()
> that use alignment properties on the returned pointer itself as part
> of the information encoding how much memory the region points to (such
> as whether the allocation comes from mmap or the heap, for example),
> and the standard should not be precluding these types of
> implementations. With such a mention in place, it may also be worth
> mentioning that when new==NULL, it is not necessary to call free(old)
> immediately, if the programmer would rather ignore the fact that the
> system cannot move the allocation to a more efficient location and
> that the tail of the old pointer is now wasted space.
Yeah, that would make sense in a footnote.
[...]
> > @@ Returns, p3
> > The <b>calloc</b> function returns
> > -either
> > a pointer to the allocated space
> > +on success.
> > -or a null pointer
> > -if
> > +If
> > the space cannot be allocated
> > or if the product <tt>nmemb * size</tt>
> > -would wraparound <b>size_t</b>.
> > +would wraparound <b>size_t</b>,
> > +a null pointer is returned.
>
> Not part of this paper, but would it make sense for implementations
> that return different errno for the two different classes of failures
> here? ENOMEM when the pointer can't be allocated, and EINVAL (or
> maybe ERANGE or EOVERFLOW) when nmemb*size overflows?
If designing realloc(3) from scratch, it could make sense. But for
backwards compatibility, we should use ENOMEM for all errors that keep
the old pointer alive.
That's because reallocarray(3) uses ENOMEM for the overflow case too,
and so, if some code depends on that, we could keep it working. I
expect the consequence wouldn't be terrible if we change that --I expect
at most a leak--, but I'm not sure; we'd have to be careful.
For example, this code would be a portable way to call reallocarray(3)
in POSIX:
errno = 0;
new = reallocarray(old, n, size);
if (new == NULL) {
if (errno == ENOMEM)
free(old);
goto fail;
}
...
free(new);
Using EOVERFLOW for n*size overflow would result in a leak of 'old' when
the size overflows.
We could decide that we can live with that leak, in return for a better
future where programs can differentiate between "OOM will trigger soon"
and "I just asked for an insane amount of memory".
Luckily, ISO C doesn't know about ENOMEM, so we have some time to talk
about it after fixing realloc(3) in ISO C.
> If C2y standardizes reallocarray(), then this becomes important.
> Without sane errno values, it is impossible to tell whether
> reallocarray() failed due to the inability to allocate the new
> pointer, or whether it failed because the parameters overflowed and no
> reallocation could be attempted. In fact, glibc documents that
> attempting to malloc(PTRDIFF_MAX+1 bytes is treated as a failure, even
> if that value is positive and less than the total amount of memory
> available in the system, because objects cannot be so large as to
> cause problems when computing pointer differences. But as long as the
> interface is documented as leaving the old pointer unchanged on ANY
> failures (whether allocation or overflow), then the semantics are
> still easy to work with even when not having errno to rely on.
Yeah, I think we have time to discuss this, as NULL will mean error, and
thus 'old' be unchanged. That is, if and only if realloc(3) returns
NULL, the original pointer should remain unchanged, per my proposal.
> > 7.25.4.7 The malloc function
> > @@ Returns, p3
> > The <b>malloc</b> function returns
> > -either
> > -a null pointer
> > -or
> > -a pointer to the allocated space.
> > +a pointer to the allocated space
> > +on success.
> > +If
> > +the space cannot be allocated,
> > +a null pointer is returned.
> >
> > 7.25.4.8 The realloc function
> > @@ Description, p2
> > The <b>realloc</b> function
> > deallocates the old object pointed to by <tt>ptr</tt>
> > +as if by a call to <b>free</b>,
> > and returns a pointer to a new object
> > -that has the size specified by <tt>size</tt>.
> > +that has the size specified by <tt>size</tt>
> > +as if by a call to <b>malloc</b>.
> > The contents of the new object
> > shall be the same as that of the old object prior to deallocation,
> > up to the lesser of the new and old sizes.
> > Any bytes in the new object
> > beyond the size of the old object
> > have unspecified values.
> >
> > @@ p3
> > If <tt>ptr</tt> is a null pointer,
> > the <b>realloc</b> function behaves
> > like the <b>malloc</b> function for the specified size.
> > Otherwise,
> > if <tt>ptr</tt> does not match a pointer
> > earlier returned by a memory management function,
> > or
> > if the space has been deallocated
> > by a call to the <b>free</b> or <b>realloc</b> function,
> > ## We can probably remove all of the above, because of the
> > ## behavior now being defined as-if by calls to malloc(3) and
> > ## free(3). But let's do that editorially in a separate change.
> > -or
> > -if the size is zero,
> > ## We're defining the behavior.
> > the behavior is undefined.
> > If
> > -memory for the new object is not allocated,
> > +the space cannot be allocated,
> > ## Editorial; for consistency with the wording of the other functions.
> > the old object is not deallocated
> > and its value is unchanged.
> >
> > @@ Returns, p4
> > The <b>realloc</b> function returns
> > a pointer to the new object
> > (which can have the same value
> > -as a pointer to the old object),
> > +as a pointer to the old object)
> > +on success.
> > -or
> > +If
> > +space cannot be allocated,
> > a null pointer
> > -if the new object has not been allocated.
> > +is returned.
>
> I'm liking the direction this proposal is headed in. If it is down to
> a question of whether glibc or the C standard will blink first, I'm
> hoping that we can get some agreement from both sides, rather than
> being stuck in a stalemate with each arguing that the other is the
> reason to not fix things.
Thanks a lot! I feel like we're closer to reach that.
Have a lovely day!
Alex
--
<https://www.alejandro-colomar.es/>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
