Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 Oct 2018 15:32:40 +0000
From:      Alexander Richardson <arichardson@freebsd.org>
To:        Michael.Tuexen@macmic.franken.de
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r339876 - head/libexec/rtld-elf
Message-ID:  <CA%2BZ_v8oBqaFa6wNkLTsZXMfdwkGmJc=eXwT13JMZJ8SNTc8k4w@mail.gmail.com>
In-Reply-To: <B4656C50-1A08-4E0E-B0F4-2DA527840679@macmic.franken.de>
References:  <201810292108.w9TL83bO041913@repo.freebsd.org> <B4656C50-1A08-4E0E-B0F4-2DA527840679@macmic.franken.de>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 30 Oct 2018 at 10:17, Michael Tuexen
<Michael.Tuexen@macmic.franken.de> wrote:
>
> > On 29. Oct 2018, at 22:08, Alex Richardson <arichardson@FreeBSD.org> wrote:
> >
> > Author: arichardson
> > Date: Mon Oct 29 21:08:02 2018
> > New Revision: 339876
> > URL: https://svnweb.freebsd.org/changeset/base/339876
> >
> > Log:
> >  rtld: set obj->textsize correctly
> >
> >  With lld-generated binaries the first PT_LOAD will usually be a read-only
> >  segment unless you pass --no-rosegment. For those binaries the textsize is
> >  determined by the next PT_LOAD. To allow both LLD and bfd 2.17 binaries to
> >  be parsed correctly use the end of the last PT_LOAD that is marked as
> >  executable instead.
> >
> >  I noticed that the value was wrong while adding some debug prints for some rtld
> >  changes for CHERI binaries. `obj->textsize` only seems to be used by PPC so the
> >  effect is untested. However, the value before was definitely wrong and the new
> >  result matches the phdrs.
> I build kernel and world with a revision later than this on a PPC. Buildword
> ends up with a world where almost all binaries are segfaulting.... Especially gdb
> (but svn, ls or so all segfault).
>
> Best regards
> Michael

This is rather surprising since if anything the range of the icache
flush should increase rather than decrease after this change.

I can only see this causing a behaviour change if we actually need to
flush more than just the executable segments.
Is it possible that some binary/library contains a non-executable
segment as the first PT_LOAD?
Or is there some linker script that adds custom PHDRS?

Alex


> >
> >  Reviewed By: kib
> >  Approved By: brooks (mentor)
> >  Differential Revision: https://reviews.freebsd.org/D17117
> >
> > Modified:
> >  head/libexec/rtld-elf/map_object.c
> >  head/libexec/rtld-elf/rtld.c
> >
> > Modified: head/libexec/rtld-elf/map_object.c
> > ==============================================================================
> > --- head/libexec/rtld-elf/map_object.c        Mon Oct 29 21:03:43 2018        (r339875)
> > +++ head/libexec/rtld-elf/map_object.c        Mon Oct 29 21:08:02 2018        (r339876)
> > @@ -93,6 +93,7 @@ map_object(int fd, const char *path, const struct stat
> >     Elf_Addr note_end;
> >     char *note_map;
> >     size_t note_map_len;
> > +    Elf_Addr text_end;
> >
> >     hdr = get_elf_header(fd, path, sb);
> >     if (hdr == NULL)
> > @@ -116,6 +117,7 @@ map_object(int fd, const char *path, const struct stat
> >     note_map = NULL;
> >     segs = alloca(sizeof(segs[0]) * hdr->e_phnum);
> >     stack_flags = RTLD_DEFAULT_STACK_PF_EXEC | PF_R | PF_W;
> > +    text_end = 0;
> >     while (phdr < phlimit) {
> >       switch (phdr->p_type) {
> >
> > @@ -130,6 +132,10 @@ map_object(int fd, const char *path, const struct stat
> >                   path, nsegs);
> >               goto error;
> >           }
> > +         if ((segs[nsegs]->p_flags & PF_X) == PF_X) {
> > +             text_end = MAX(text_end,
> > +                 round_page(segs[nsegs]->p_vaddr + segs[nsegs]->p_memsz));
> > +         }
> >           break;
> >
> >       case PT_PHDR:
> > @@ -280,8 +286,7 @@ map_object(int fd, const char *path, const struct stat
> >     }
> >     obj->mapbase = mapbase;
> >     obj->mapsize = mapsize;
> > -    obj->textsize = round_page(segs[0]->p_vaddr + segs[0]->p_memsz) -
> > -      base_vaddr;
> > +    obj->textsize = text_end - base_vaddr;
> >     obj->vaddrbase = base_vaddr;
> >     obj->relocbase = mapbase - base_vaddr;
> >     obj->dynamic = (const Elf_Dyn *) (obj->relocbase + phdyn->p_vaddr);
> >
> > Modified: head/libexec/rtld-elf/rtld.c
> > ==============================================================================
> > --- head/libexec/rtld-elf/rtld.c      Mon Oct 29 21:03:43 2018        (r339875)
> > +++ head/libexec/rtld-elf/rtld.c      Mon Oct 29 21:08:02 2018        (r339876)
> > @@ -1390,13 +1390,15 @@ digest_phdr(const Elf_Phdr *phdr, int phnum, caddr_t e
> >           if (nsegs == 0) {   /* First load segment */
> >               obj->vaddrbase = trunc_page(ph->p_vaddr);
> >               obj->mapbase = obj->vaddrbase + obj->relocbase;
> > -             obj->textsize = round_page(ph->p_vaddr + ph->p_memsz) -
> > -               obj->vaddrbase;
> >           } else {            /* Last load segment */
> >               obj->mapsize = round_page(ph->p_vaddr + ph->p_memsz) -
> >                 obj->vaddrbase;
> >           }
> >           nsegs++;
> > +         if ((ph->p_flags & PF_X) == PF_X) {
> > +             obj->textsize = MAX(obj->textsize,
> > +                 round_page(ph->p_vaddr + ph->p_memsz) - obj->vaddrbase);
> > +         }
> >           break;
> >
> >       case PT_DYNAMIC:
> >
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BZ_v8oBqaFa6wNkLTsZXMfdwkGmJc=eXwT13JMZJ8SNTc8k4w>