Bug 12547

Summary: realloc(p, 0) violates C99
Product: glibc Reporter: Martin Sebor <msebor>
Component: mallocAssignee: Ulrich Drepper <drepper.fsp>
Status: RESOLVED INVALID    
Severity: normal CC: bruno, bugdal, eblake, fweimer, jakub, nick
Priority: P2 Flags: fweimer: security-
Version: 2.13   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Martin Sebor 2011-03-06 22:48:33 UTC
The C99 standard specifies that an implementation may return NULL from a call to realloc(p, 0). However, in such a case a conforming implementation must avoid freeing the space pointed to by p and a program must call free(p). The test case below shows that glibc violates this C99 requirement, causing a program to free the space twice. Note that this C99 requirement to avoid freeing the pointer is in contrast to POSIX -- see the discussion on the austin-group-l list starting with this post:
https://www.opengroup.org/sophocles/show_mail.tpl?CALLER=show_archive.tpl&source=L&listname=austin-group-l&id=15252

$ cat <<EOF | gcc -xc - && ./a.out 
#include <stdlib.h>

int main(void) {
    void *p, *q;

    p = malloc(1);
    q = realloc(p, 0);

    if (p && !q)
        free(p);

    return 0;
}
EOF
*** glibc detected *** double free or corruption (fasttop): 0x0000000000501010 ***
Aborted
Comment 1 Ulrich Drepper 2011-03-11 01:11:45 UTC
There is no way this will be changed.  This is how it has been implemented forever.  C should document existing practice.  Changing it would mean introducing memory leaks.
Comment 2 Joseph Myers 2011-03-17 19:02:55 UTC
In view of the conclusion at the WG14 meeting today that C1X should stay with the C99 semantics for realloc rather than reverting to the C90/POSIX semantics, and the consequentially proposed changes for POSIX
http://austingroupbugs.net/view.php?id=374
this looks rather like a case for having different versions of realloc depending on feature test macros (with strict conformance to newer POSIX versions, or to C99 or C1X without features beyond ISO C, getting the C99 version, and other modes (and existing binaries) getting the C90 version).
Comment 3 Eric Blake 2011-03-24 17:09:54 UTC
http://austingroupbugs.net/view.php?id=400 has been created to track the resolution of fixing POSIX to no longer conflict with C99; glibc input is necessary to help determine the correct way to resolve the POSIX dilemma.
Comment 4 Eric Blake 2011-03-24 17:19:57 UTC
(In reply to comment #2)
> In view of the conclusion at the WG14 meeting today that C1X should stay with
> the C99 semantics for realloc rather than reverting to the C90/POSIX semantics,
> and the consequentially proposed changes for POSIX
> http://austingroupbugs.net/view.php?id=374
> this looks rather like a case for having different versions of realloc
> depending on feature test macros (with strict conformance to newer POSIX
> versions, or to C99 or C1X without features beyond ISO C, getting the C99
> version, and other modes (and existing binaries) getting the C90 version).

Given that glibc has already done this for other interfaces (witness __xpg_strerror_r), I think it would be appropriate for a glibc patch that introduced __xpg_realloc for strict compliance.
Comment 5 Jakub Jelinek 2011-03-24 17:23:15 UTC
For realloc that is more difficult, as realloc is often interposed.
Plus it is going to break lots of programs, which assume realloc (x, 0) is like free (x).  I don't think we should change glibc.
Comment 6 Bruno Haible 2011-03-25 00:08:12 UTC
The behaviour of realloc(p, 0) is the same on Solaris 10, AIX 7.1,
HP-UX 11.11, OSF/1 5.1, mingw as on glibc systems.

The other behaviour (realloc(p, 0) returning normally non-NULL) is
present on FreeBSD, OpenBSD, Cygwin, IRIX.
Comment 7 Eric Blake 2011-04-01 22:51:34 UTC
We can't just blindly ignore the WG14 C committee - either we have to fight them harder to get the C1x standard (now in draft balloting) fixed, or we'll have to give in and implement something like __xpg_realloc :(

Would anyone be willing to implement a systemtap probe to find out how many programs in real life actually use realloc(p,0) in the first place, to see the extent of the damage control if we are forced to change semantics?



Sent to the SC22WG14 email reflector:

The Austin Group has been studying this problem (see http://austingroupbugs.net/view.php?id=400) [^] and trying to resolve any conflict with C. This was also examined by WG14 during the London meeting, where the resolution was "POSIX should conform to the requirements of C". The Austin Group agrees with this resolution.

However, during our discussions, we have found the following in the C standard (using refs to C99, not C1x):

Point 1: Objects cannot be zero sized.
6.2.6.1(2) Except for bit-fields, objects are composed of contiguous sequences of one or more bytes,

Point 2: realloc(NULL, 0) is required to behave like malloc(0), and malloc states "7.20.3.3(2) The malloc function allocates space for an object whose size is specified by size and whose value is indeterminate." Since objects cannot be zero sized, returning a non-null pointer is at best questionable. However, this argues that realloc(p, 0) should always fail.

Point 3: Explicit permission to succeed AND return NULL is granted: "7.20.3(1) If the size of the space requested is zero, the behavior is implementation- defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object." [this refers to calloc, malloc and realloc]. It has been claimed that if realloc(p,0) returns a null pointer, that indicates that "the new object cannot be allocated" and so the old object remains alive. However, that interpretation is not clear, and the two separate sentences in 7.20.3.1 about null pointers are distinct cases, and the second does not necessarily imply the first.

The POSIX words currently follow the original C89 text, and not the C99 update. They state: "If size is 0 and ptr is not a null pointer,
the object pointed to is freed. ...RETURNS... If size is 0, either a null pointer or a unique pointer that can be successfully passed to free() shall be returned." The C99 words state "7.20.3.4(2) The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size." This would appear to allow an implementation to deallocate the space and then return NULL (without setting errno). Additionally, "7.20.3(3) ... or if the space has been deallocated by a call to the free or realloc function..." strongly suggests that realloc(ptr, 0) is a valid way to free ptr.

So, the Austin Group currently does not believe that POSIX conflicts with C, and that it is permissible for realloc(p, 0) (where p is non-NULL) to free the space allocated and return NULL. In POSIX, it is acceptable to set errno to zero before the call, and test it afterwards to see if the pointer has been freed or not.

Our current survey has shown that the following implementations free the pointer and return NULL :

Glibc (GNU/Linux)
AIX
HP-UX
Solaris
OSF/1

There is strong resistance from at least one of these to changing their implementation, in the belief that this would make current, memory leak free, conforming code become both non-conforming and leaking.

-- 
Nick Stoughton
Austin Group/WG14 Liaison


	Reply on SC22WG14 reflector:

What WG 14 has been saying is that that interpretation is
incorrect. Referencing the latest C1x draft:

1. 7.22.3.5p4, "Returns," states "The realloc function returns a
pointer to the new object (which may have the same value as a pointer to
the old object), or a null pointer if the new object could not be
allocated." In other words, null means failure.

2. 7.22.3p1 states "If the space cannot be allocated, a null pointer is
returned. If the size of the space requested is zero, the behavior is
implementation-defined: either a null pointer is returned, or the
behavior is as if the size were some nonzero value, except that the
returned pointer shall not be used to access an object." This, together
with the previous quote, means that the function can either succeed and
return a valid pointer, or fail and return a null pointer.

3. 7.22.3.5p1 states "If memory for the new object cannot be allocated,
the old object is not deallocated and its value is unchanged." This,
together with the previous two quotes, means that if a null pointer is
returned, the operation has failed and the old object is not deallocated.

      You mentioned the fact that objects cannot be zero sized. That is
why the wording is the way it is. If we had zero-sized objects, we
could just say that an allocation of size zero returns a pointer to just
past the end of a zero-sized object (and that is why the pointer cannot
be dereferenced). Instead, we have to say the same thing without
talking about zero-sized objects.
Comment 8 Ulrich Drepper 2011-04-01 23:40:23 UTC
> We can't just blindly ignore the WG14 C committee

Yes, we can.

This is not the first time standards committees do something stupid and not the first time they get ignored.
Comment 9 Jackie Rosen 2014-02-16 17:51:42 UTC Comment hidden (spam)
Comment 10 Rich Felker 2015-02-22 20:49:43 UTC
Reopening. Can this be reconsidered now that decision making is consensus based and non-hostile to standards?
Comment 11 jsm-csl@polyomino.org.uk 2015-02-23 12:19:02 UTC
The proposed TC for DR#400 (closed, presumably to be published in C11 TC2) 
makes the standard say "If size is zero and memory for the new object is 
not allocated, it is implementation-defined whether the old object is 
deallocated. ... The realloc function returns a pointer to the new object 
(which may have the same value as a pointer to the old object), or a null 
pointer if the new object has not been allocated.  ... Invoking realloc 
with a size argument equal to zero is an obsolescent feature.".

Based on that conclusion about what realloc may do in this case, this 
sounds like INVALID to me.
Comment 12 Rich Felker 2015-02-23 18:09:52 UTC
I don't think it's so clear-cut. While the committee's decision to enshrine this ugly historical behavior as allowable implementation-defined behavior means glibc will be conforming to C11+TC2, conformance with older versions of the standard (which glibc usually aims for) is left unresolved. But more importantly, the committee's decision to make realloc(p,0) implementation-defined and obsolescent means applications should stop using it, because it's unsafe; therefore, I think glibc's priority should be minimizing the danger.

Programs that expect glibc's historical behavior will experience only memory leaks if compiled and run on systems that conform to C99 or present C11. On the other hand, programs that expect the standard C99 behavior but run on current glibc will exhibit double-free errors, a major source of security flaws. Being that correct programs should not be using realloc(p,0) anyway, I think memory leaks are a much lesser evil than double-free.
Comment 13 Bruno Haible 2015-02-27 18:11:01 UTC
> I think memory leaks are a much lesser evil than double-free.

I disagree. Double-free errors get noticed and reported and therefore fixed rather quickly. Memory leaks don't. => They are the longer-lasting evil.

But since the standard will say "Invoking realloc with a size argument equal to zero is an obsolescent feature." glibc could give a warning about this case, if some debugging flag is enabled (e.g. mtrace or valgrind?).
Comment 14 Rich Felker 2015-02-27 19:11:31 UTC
Double-free is not easily detected except in the case where both frees take place in sequence with no intervening allocations. For example, in a case like this:

void *p = malloc(n);
realloc(p, 0);
void *q = malloc(n);
free(p);

it's very likely that line 4 will end up freeing the allocation made in line 3. The resulting state is extremely dangerous and almost always leads to arbitrary code execution if enough effort is put into the analysis.

Memory leaks, on the other hand, are at worst a DoS issue.

Marking of a feature as "obsolescent" does not grant the implementation permission to do crazy things like printing messages. It's purely a warning to applications that the interface may be removed, or its behavior may become undefined, in future versions of the language standard. Code using it is still perfectly valid and conforming to the current language standard.
Comment 15 Florian Weimer 2018-05-09 12:22:46 UTC
I believe this is now being treated as a defect in the C standard:

http://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm#dr_400

The deallocating behavior implemented in glibc seems to conform with one alternative in the new wording.  The other (non-deallocating) alternative does not match POSIX, but that's not a problem for glibc malloc.
Comment 16 Florian Weimer 2018-05-14 08:14:19 UTC
As I implied in comment 15, the current POSIX specification permits the current glibc behavior, especially if we change free not to set errno (bug 17924).

I think the future version of the ISO C standard is still secret, so I cannot comment on it here, but I suggest to disregard it anyway: We are not going to change behavior again merely for the sake of standards compliance, so I'm closing this bug.
Comment 17 Rich Felker 2019-10-03 11:58:42 UTC
FYI the identical issue in Bionic has resulted in a RCE in WhatsApp:

https://awakened1712.github.io/hacking/hacking-whatsapp-gif-rce/