From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 2 Apr 2012 01:45:28 +0200 Subject: [U-Boot] [PATCH] Prevent malloc with size 0 In-Reply-To: References: <4CC006B1.8000905@intracomdefense.com> <201204011621.46057.marek.vasut@gmail.com> Message-ID: <201204020145.28167.marek.vasut@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Graeme Russ, > Hi All > > Here we go again ;) Yay (polishing my flamethrower)! > On Mon, Apr 2, 2012 at 12:21 AM, Marek Vasut wrote: > > Dear Joakim Tjernlund, > > > >> Marek Vasut wrote on 2012/04/01 16:01:56: > >> > Dear Joakim Tjernlund, > >> > > >> > > > Dear Mike Frysinger, > >> > > > > >> > > > > On Thursday, October 21, 2010 17:10:31 Graeme Russ wrote: > >> > > > > > On 22/10/10 06:51, Mike Frysinger wrote: > >> > > > > > > have u-boot return an error. > >> > > > > > > >> > > > > > Is NULL what you consider to be an error > >> > > > > > >> > > > > yes > >> > > > > > >> > > > > > Besides, is not free(NULL) valid (does nothing) as well? > >> > > > > > >> > > > > yes, free(NULL) should work fine per POSIX > >> > > > > -mike > >> > > > > >> > > > Well then, this patch wasn't accepted yet and I consider it OK to > >> > > > apply. Any objections? > >> > > > >> > > There was a long debate on the list regarding this where I argued > >> > > that malloc(0) should not be an error and malloc should return a > >> > > ptr != NULL I guess that is why it hasn't been applied. > >> > > > >> > > Jocke > >> > > >> > Ok, let's restart. Is there any objection why malloc(0) should not > >> > return NULL in uboot? > >> > >> Yes, read the thread to see why. > > > > Well I did, that's why I have no objections to applying this patch > > > >> > Is it coliding with any spec? > >> > >> No, both are valid. > > > Out of principle I would say that malloc(0) should return a non-NULL > pointer of an area where exactly 0 bytes may be used. And, of course, > free() of that area shall not fail or crash the system. > > > I'm wondering how exactly this would work - In theory, if you tried to > access this pointer you should get a segv. But I suppose if you malloc(1) > and try to access beyond the first byte there probably won't be a segv > either.... > > So to review the facts: > > - The original complaint was that malloc(0) corrupts the malloc data > structures, not that U-Boot's malloc(0) behaviour is non-standard > - Both the malloc(0) returns NULL and malloc(0) returns a uniquely > free'able block of memory solutions are standard compliant > - malloc(0) returning NULL may break code which, for the sake of code > simplicity, does not bother to check for zero-size before calling > malloc() Well but you said malloc(0) corrupts the mallocator's data structures. Therefore malloc(0) used in code right now is broken anyway. > - malloc(0) returning NULL may help to identify brain-dead use-cases Agreed. > > My vote: > > if ((long)bytes == 0) { > DEBUG("Warning: malloc of zero block size\n"); > bytes = 1; Well ... no, how can malloc(0) returning NULL break code that's already broken any more? It's silently roughing the mallocator structures up and it means the code is sitting on a ticking a-bomb anyway. So we should add this like: if (bytes == 0) { debug("You're sitting on a ticking A-Bomb doing this"); return NULL; } else if (bytes < 0) { return NULL; } > } else if ((long)bytes < 0) { > DEBUG("Error: malloc of negative block size\n"); > return 0; > } > > Regards, > > Graeme Best regards,