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

[Timo Teras <timo.teras@iki.fi>]

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.

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.

> > This is not exactly intended to be committed, but shows clearly
> > what is wrong with the current implementation.
> 
> Not so clear. :)

Hope the above explains the root problem.

> 
> > 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.

- Timo