Re: [PATCH 3/3] [FYI] fix dynamic linker dso loading

[Richard Felker <dalias@aerifal.cx>]

On Wed, Jul 10, 2013 at 06:47:25PM +0300, Timo Teras wrote:
> On Wed, 10 Jul 2013 11:00:03 -0400
> Rich Felker <dalias@aerifal.cx> wrote:
> 
> > On Wed, Jul 10, 2013 at 04:39:01PM +0300, Timo Teräs wrote:
> > > The phdr entries need to be allocated from heap, so later calls
> > > to dl_iterate_phdr work properly. Make sure the ARM unwind info
> > > is not freed.
> > 
> > I am confused about the motivation for this patch. The program headers
> > are part of the mapping and are never freed.
> 
> static void *map_library(int fd, struct dso *dso)
> {
> 	Ehdr buf[(896+sizeof(Ehdr))/sizeof(Ehdr)];
> ....
> 	ssize_t l = read(fd, buf, sizeof buf);
> ....
> 	ph = (void *)((char *)buf + eh->e_phoff);
> ....
> 	dso->phdr = ph;
> 
> So no, the program headers are not part of the mapping. At least they
> are not setup that way currently.

Indeed, this is purely my fault for failing to review this part of the
patch when it was committed. I was not aware that dso->phdr was being
pointed to the wrong memory; presumably it "happened to work" for some
tests I did at the time. I will fix it.

> Instead dso->phdr points to stack and gets messed up. That's why the:
> -	dso->phdr = ph;
> +	dso->phdr = malloc(phsize);
> +	memcpy(dso->phdr, ph, phsize);
> 
> Perhaps the proper fix would be to map them instead then.

They are already mapped anyway; the pointer into the right offset of
the map is just not setup.

> > > The reclamation fix should be probably something better, as I
> > > believe the same applies to GNU_EH_FRAME phdr.
> > 
> > It definitely does not apply to GNU_EH_FRAME.
> 
> Seems I misunderstood in hurry what the reclaim_gaps really does.
> Probably one of the reasons why it has the "huge hack" comment.. :)
> 
> I believe the "ph->p_type != PT_ARM_EXIDX" additions are not needed
> after all.

Indeed, all that's needed is the correct pointer value. I'll get a
patch committed soon that should fix the issue; please let me know if
other issues persist.

Rich