This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #15670] Replace alloca in __tzfile_read by malloc.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Mike Frysinger <vapier at gentoo dot org>
- Cc: libc-alpha at sourceware dot org, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Mon, 14 Oct 2013 20:24:09 +0200
- Subject: Re: [PATCH][BZ #15670] Replace alloca in __tzfile_read by malloc.
- Authentication-results: sourceware.org; auth=none
- References: <20131014131522 dot GA23174 at domone dot podge> <Pine dot LNX dot 4 dot 64 dot 1310141359220 dot 11037 at digraph dot polyomino dot org dot uk> <20131014144952 dot GA23875 at domone dot podge> <201310141353 dot 21699 dot vapier at gentoo dot org>
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 going to switch to malloc, then it seems like using asprintf would
> be simpler:
> if (asprintf (&new, "%s/%s", tzdir, file))
> goto ret_free_transitions;
ti would,