[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130710165223.GA29800@brightrain.aerifal.cx>
Date: Wed, 10 Jul 2013 12:52:23 -0400
From: Richard Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 3/3] [FYI] fix dynamic linker dso loading
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@...ifal.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
