[U-Boot] [PATCH] Prevent malloc with size 0

[Marek Vasut]

Dear Graeme Russ,

> Hi Marek,
> 
> On Mon, Apr 2, 2012 at 11:04 AM, Marek Vasut <marek.vasut at gmail.com> wrote:
> > Dear Graeme Russ,
> > 
> >> Hi Marek,
> >> 
> >> On Mon, Apr 2, 2012 at 10:13 AM, Marek Vasut <marek.vasut at gmail.com> wrote:
> >> > Dear Graeme Russ,
> >> > 
> >> >> Hi Marek,
> >> >> 
> >> >> On Mon, Apr 2, 2012 at 9:45 AM, Marek Vasut <marek.vasut at gmail.com> 
wrote:
> >> >> > Dear Graeme Russ,
> >> >> 
> >> >> Because you just set it off - Right now, that code is assuming
> >> >> malloc(0) will return a valid pointer and thus not throw an E_NOMEM
> >> >> error - Now all that code will fail with E_NOMEM
> >> > 
> >> > Well ... that code worked with invalid memory (most probably not even
> >> > R/W because it was some completely random hunk) and worked only by
> >> > sheer coincidence. Let's break it, it was broken anyway.
> >> 
> >> a) The code calling malloc(0) is not broken, U-Boot's implementation of
> >>    malloc(0) is.
> > 
> > Well if it corrupts the internal structures of the mallocator, it's
> > broken because it works by sheer coincidence. But I know what you wanna
> > point out.
> 
> If I call printf() with incorrect format specifiers and arguments (and the
> compile does not pick it up) then my code is broken. If I call printf() and
> the systems implementation does not support a standard format specifier
> that I'm using then printf() is broken, not my code.
> 
> malloc(0) is a permissible call - It's unfortunate that the behaviour is
> unspecified:
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf
> ISO/IEC 9899:TC2 - May 6, 2005
> 
> 7.20.3.3 The malloc function (p.314)
>   The malloc function allocates space for an object whose size is specified
>   by size and whose value is indeterminate.
> 
>  Returns
>   The malloc function returns either a null pointer or a pointer to the
>   allocated space.
> 
> J.1 Unspecified behavior (p. 490)
>  - The amount of storage allocated by a successful call to the calloc,
>    malloc, or realloc function when 0 bytes was requested (7.20.3)
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
> ISO/IEC 9899:201x - April 12, 2011
> 
> 7.22.3.4 The malloc function (p. 349)
>  Synopsis
>   #include <stdlib.h>
>   void *malloc(size_t size);
>  Description
>   The malloc function allocates space for an object whose size is specified
>   by size and whose value is indeterminate.
>  Returns
>   The malloc function returns either a null pointer or a pointer to the
>   allocated space.
> 
> J.1 Unspecified behavior (p. 556)
>  The amount of storage allocated by a successful call to the calloc,
>  malloc, or realloc function when 0 bytes was requested (7.22.3).
> 
> I have also seen forum postings of the form:
>  Section 7.20.3
>   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.
> 
> but have not seen an official document stating this
> 
> >> b) The code calling malloc(0) is making a perfectly legitimate
> >> assumption based on how glibc handles malloc(0)
> > 
> > Yes, agreed
> > 
> >> c) Just because glibc does something does not mean we have to
> > 
> > ACK
> > 
> >> d) malloc(0) returning NULL and malloc(0) returning a valid pointer is
> >> not going to trouble me as I will never call malloc(0)
> > 
> > You sure? :)
> 
> No ;)
> 
> > Anyway, if we return something else than 0, how are we gonna trap such a
> > null pointer?
> 
> You don't - You ask for a pointer to a block of memory of zero size and
> malloc will return the smallest block it can. Remember, malloc(x) does not
> have to return a block of exactly x bytes - it must return a block of at
> least x bytes. It is up to you not to deference the pointer passed back
> from malloc(0)
> 
> >> > Do you know about any such code? That's why I suggest adding such a
> >> > debug() only in case there's malloc(0) called. Maybe even add a
> >> > printf() instead.
> >> 
> >> Did you see the FDT example - Admitedly not in U-Boot but it's a really
> >> good example IMHO - For the sake of code simplisity and clarity, some
> >> processing loops are best implemented assuming malloc(0) will return
> >> a valid pointer. Now if that pointer is de-referenced, then that is
> >> the callers problem...
> > 
> > I did not see it, where?
> 
> patchwork (http://patchwork.ozlabs.org/patch/71914/)
> 
> Bottom line is, we could do either and we would be 100% compliant with the
> C standard
> 
> The question is, what would be more onerous. Since the majority of U-Boot
> developers will be more familiar with the glibc implementation, we may
> one day end up with code that blindly assumes malloc(0) returns a valid
> pointer and not NULL so to me, returning a valid pointer would be a logical
> choice.
> 
> On the converse, returning NULL from malloc(0) means that any attempt to
> (illegally) deference it will be immediately obvious...

So it's a question of being fool-proof vs. being compatible with glibc. This is 
a tough one, so what about voting ? ;-)

> Regards,
> 
> Graeme