Re: [PATCH][BZ #15670] Replace alloca in __tzfile_read by malloc.

[Mike Frysinger <vapier at gentoo dot org>]

On Monday 14 October 2013 14:24:09 OndÅej BÃlka wrote:
> On Mon, Oct 14, 2013 at 01:53:19PM -0400, Mike Frysinger wrote:
> > On Monday 14 October 2013 10:49:52 OndÅej BÃlka wrote:
> > > On Mon, Oct 14, 2013 at 02:03:57PM +0000, Joseph S. Myers wrote:
> > > > On Mon, 14 Oct 2013, Ondrej Bilka wrote:
> > > > > This is one of bugs that take longer to read than to fix. There is
> > > > > a unbound alloca and obvious limit is PATH_MAX.
> > > > 
> > > > This also doesn't deal with the point in the bug that strlen (either
> > > > strlen) could overflow the "unsigned int" variables.
> > > > 
> > > > You need to change both variables to size_t and check
> > > > __libc_use_alloca to determine whether to use alloca or malloc.
> > > 
> > > And what is alloca doing there anyway? This code is in no way
> > > performance critical so malloc should suffice.
> > > ...
> > > @@ -157,7 +157,7 @@ __tzfile_read (const char *file, size_t extra, char
> > > **extrap) else
> > >  	tzdir_len = strlen (tzdir);
> > >        len = strlen (file) + 1;
> > > -      new = (char *) __alloca (tzdir_len + 1 + len);
> > > +      new = (char *) malloc (tzdir_len + 1 + len);
> > 
> > and what do you do when (tzdir_len+1+len) overflows ?
> 
> On which system is this possible? Sum of their sizes is less than physical
> memory and three bytes do not make difference.

if you're worried about the size being too large to alloca, then i don't 
really see why malloc would be any better here.  you still haven't checked the 
return value of malloc.

using the suggested asprintf code would fix that (among other things).
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.