Re: svn commit: r339876 - head/libexec/rtld-elf

2018-10-30 Thread Konstantin Belousov
On Tue, Oct 30, 2018 at 03:32:40PM +, Alexander Richardson wrote:
> On Tue, 30 Oct 2018 at 10:17, Michael Tuexen
>  wrote:
> >
> > > On 29. Oct 2018, at 22:08, Alex Richardson  
> > > 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?
> 
Could it be that there is a hole between start of the object mapping and
the last PT_LOADable segment eligible for execution ?

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r339876 - head/libexec/rtld-elf

2018-10-30 Thread Alexander Richardson
On Tue, 30 Oct 2018 at 10:17, Michael Tuexen
 wrote:
>
> > On 29. Oct 2018, at 22:08, Alex Richardson  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.cMon Oct 29 21:03:43 2018  
> >   (r339875)
> > +++ head/libexec/rtld-elf/map_object.cMon 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:
> >
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r339876 - head/libexec/rtld-elf

2018-10-30 Thread Michael Tuexen
> On 29. Oct 2018, at 22:08, Alex Richardson  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
> 
>  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.cMon Oct 29 21:03:43 2018
> (r339875)
> +++ head/libexec/rtld-elf/map_object.cMon 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:
> 

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r339876 - head/libexec/rtld-elf

2018-10-29 Thread Alex Richardson
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.
  
  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.cMon Oct 29 21:03:43 2018
(r339875)
+++ head/libexec/rtld-elf/rtld.cMon 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:
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"