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

[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...

Regards,

Graeme