Re: [PATCH] Fix invalid free of memory allocated during rtld init

["Dmitry V. Levin" <ldv at altlinux dot org>]

On Tue, Jan 15, 2013 at 11:10:00PM -0500, Carlos O'Donell wrote:
> On 01/15/2013 11:04 AM, Andreas Schwab wrote:
> > Any memory allocated during rtld init cannot be freed.
> > 
> > Andreas.
> > 
> > 	[BZ #14293]
> > 	* elf/dl-load.c (_dl_init_paths): Mark decomposed RUNPATH as
> > 	non-freeable.
> > 
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index 6e65980..a3aab8a 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -797,6 +797,7 @@ _dl_init_paths (const char *llp)
> >  			   (const void *) (D_PTR (l, l_info[DT_STRTAB])
> >  					   + l->l_info[DT_RUNPATH]->d_un.d_val),
> >  			   l, "RUNPATH");
> > +	  l->l_runpath_dirs.malloced = 0;
> >  
> >  	  /* The RPATH is ignored.  */
> >  	  l->l_rpath_dirs.dirs = (void *) -1;
> > 
> 
> As you work on the problem you familiarize yourself
> with the code in question and it seems obvious to you that
> this one line change is the solution to the problem.
> 
> For example those familiar with the code know that malloc
> can't be called during early relocation, instead we use
> a dummy allocator for which we can't call free later.
> 
> Unfortunately, even to the reviewer, me, it doesn't
> make it any easier to review because you have not provided 
> sufficient information to help me review. In particular
> I need three things:
> (a) assumptions, 
> (b) analysis and 
> (c) conclusions.
> 
> Stating (c) directly without (a) or (b) means that I as 
> the reviewer need to create (a) and (b) *without* your 
> help and compare them to (c). This increases the burden
> on the reviewer significantly.
> 
> Could you please help by posting (a) and (b)?

(a) every successful decompose_rpath() invocation during rtld init
must be followed by clearing of "malloced" flag;
(b) _dl_init_paths() fails to accomplish that.

You just need a bit more context to see that the fix is obviously correct. :)


-- 
ldv

Attachment: pgp00000.pgp
Description: PGP signature