Re: ld.so diff that needs testing on landisk

2017-01-05 Thread Mark Kettenis 
> From: Philip Guenther 
> Date: Wed, 4 Jan 2017 17:21:03 -0700
> 
> On Tue, Jan 3, 2017 at 2:13 PM, Mark Kettenis  wrote:
> > The diff below (partly by guenther@) removes ld.so's dependency on the
> > __got_{start,end} symbols by looking at PT_GNU_RELRO instead.  On some
> > platforms (hppa and perhaps a few others) this leads to even less
> > writable pages.  However, we're not sure if this will work correctly
> > on landisk.  So if somebody with a fairly up-to-date landisk could
> > give this a spin for us, it would be highly appreciated.
> 
> Looks like one change got lost in the back-and-forth...
> 
> 
> > --- libexec/ld.so/boot.c13 Aug 2016 20:57:04 -  1.14
> > +++ libexec/ld.so/boot.c2 Jan 2017 15:55:52 -
> ...
> > @@ -189,4 +191,30 @@ _dl_boot_bind(const long sp, long *dl_da
> >  * we have been fully relocated here, so most things no longer
> >  * need the loff adjustment
> >  */
> > +
> > +   /*
> > +* No further changes to the PLT and/or GOT are needed so make
> > +* them read-only.
> > +*/
> > +
> > +   /* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO 
> > */
> > +   ehdp = (Elf_Ehdr *)loff;
> > +   phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
> > +   for (i = 0; i < dl_data[AUX_phnum]; i++, phdp++) {
> 
> I believe this line should be:
> +   for (i = 0; i < ehdp->e_phnum; i++, phdp++) {

Oops.  Right.  I messed that up.

New diff below.  Tests, especially on landisk still appreciated.

Index: libexec/ld.so/boot.c
===
RCS file: /cvs/src/libexec/ld.so/boot.c,v
retrieving revision 1.14
diff -u -p -r1.14 boot.c
--- libexec/ld.so/boot.c13 Aug 2016 20:57:04 -  1.14
+++ libexec/ld.so/boot.c5 Jan 2017 10:32:28 -
@@ -87,6 +87,8 @@ _dl_boot_bind(const long sp, long *dl_da
longloff;
Elf_Addri;
RELOC_TYPE  *rp;
+   Elf_Ehdr*ehdp;
+   Elf_Phdr*phdp;
 
/*
 * Scan argument and environment vectors. Find dynamic
@@ -189,4 +191,30 @@ _dl_boot_bind(const long sp, long *dl_da
 * we have been fully relocated here, so most things no longer
 * need the loff adjustment
 */
+
+   /*
+* No further changes to the PLT and/or GOT are needed so make
+* them read-only.
+*/
+
+   /* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO */
+   ehdp = (Elf_Ehdr *)loff;
+   phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
+   for (i = 0; i < ehdp->e_phnum; i++, phdp++) {
+   switch (phdp->p_type) {
+#if defined(__alpha__) || defined(__hppa__) || defined(__powerpc__) || \
+defined(__sparc64__)
+   case PT_LOAD:
+   if ((phdp->p_flags & (PF_X | PF_W)) != (PF_X | PF_W))
+   break;
+   _dl_mprotect((void *)(phdp->p_vaddr + loff),
+   phdp->p_memsz, PROT_READ);
+   break;
+#endif
+   case PT_GNU_RELRO:
+   _dl_mprotect((void *)(phdp->p_vaddr + loff),
+   phdp->p_memsz, PROT_READ);
+   break;
+   }
+   }
 }
Index: libexec/ld.so/loader.c
===
RCS file: /cvs/src/libexec/ld.so/loader.c,v
retrieving revision 1.167
diff -u -p -r1.167 loader.c
--- libexec/ld.so/loader.c  28 Aug 2016 04:33:17 -  1.167
+++ libexec/ld.so/loader.c  5 Jan 2017 10:32:28 -
@@ -413,25 +413,6 @@ _dl_boot(const char **argv, char **envp,
 #define ROUND_PG(x) (((x) + align) & ~(align))
 #define TRUNC_PG(x) ((x) & ~(align))
 
-   /*
-* now that GOT and PLT have been relocated, and we know
-* page size, protect them from modification
-*/
-#ifndef  RTLD_NO_WXORX
-   {
-   extern char *__got_start;
-   extern char *__got_end;
-
-   if (&__got_start != &__got_end) {
-   _dl_mprotect((void *)ELF_TRUNC((long)&__got_start,
-   _dl_pagesz),
-   ELF_ROUND((long)&__got_end,_dl_pagesz) -
-   ELF_TRUNC((long)&__got_start, _dl_pagesz),
-   GOT_PERMS);
-   }
-   }
-#endif
-
_dl_setup_env(argv[0], envp);
 
DL_DEB(("rtld loading: '%s'\n", __progname));
Index: libexec/ld.so/alpha/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/alpha/archdep.h,v
retrieving revision 1.17
diff -u -p -r1.17 archdep.h
--- libexec/ld.so/alpha/archdep.h   6 Dec 2015 23:36:12 -   1.17
+++ libexec/ld.so/alpha/archdep.h   5 Jan 2017 10:32:28 -
@@ -65,6 +65,4 @@ RELOC_DYN(Elf64_Rela *r, const Elf64_Sym

Re: ld.so diff that needs testing on landisk

2017-01-04 Thread Philip Guenther 
On Tue, Jan 3, 2017 at 2:13 PM, Mark Kettenis  wrote:
> The diff below (partly by guenther@) removes ld.so's dependency on the
> __got_{start,end} symbols by looking at PT_GNU_RELRO instead.  On some
> platforms (hppa and perhaps a few others) this leads to even less
> writable pages.  However, we're not sure if this will work correctly
> on landisk.  So if somebody with a fairly up-to-date landisk could
> give this a spin for us, it would be highly appreciated.

Looks like one change got lost in the back-and-forth...


> --- libexec/ld.so/boot.c13 Aug 2016 20:57:04 -  1.14
> +++ libexec/ld.so/boot.c2 Jan 2017 15:55:52 -
...
> @@ -189,4 +191,30 @@ _dl_boot_bind(const long sp, long *dl_da
>  * we have been fully relocated here, so most things no longer
>  * need the loff adjustment
>  */
> +
> +   /*
> +* No further changes to the PLT and/or GOT are needed so make
> +* them read-only.
> +*/
> +
> +   /* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO */
> +   ehdp = (Elf_Ehdr *)loff;
> +   phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
> +   for (i = 0; i < dl_data[AUX_phnum]; i++, phdp++) {

I believe this line should be:
+   for (i = 0; i < ehdp->e_phnum; i++, phdp++) {


Philip Guenther

ld.so diff that needs testing on landisk

2017-01-03 Thread Mark Kettenis 
The diff below (partly by guenther@) removes ld.so's dependency on the
__got_{start,end} symbols by looking at PT_GNU_RELRO instead.  On some
platforms (hppa and perhaps a few others) this leads to even less
writable pages.  However, we're not sure if this will work correctly
on landisk.  So if somebody with a fairly up-to-date landisk could
give this a spin for us, it would be highly appreciated.


Index: libexec/ld.so/boot.c
===
RCS file: /cvs/src/libexec/ld.so/boot.c,v
retrieving revision 1.14
diff -u -p -r1.14 boot.c
--- libexec/ld.so/boot.c13 Aug 2016 20:57:04 -  1.14
+++ libexec/ld.so/boot.c2 Jan 2017 15:55:52 -
@@ -87,6 +87,8 @@ _dl_boot_bind(const long sp, long *dl_da
longloff;
Elf_Addri;
RELOC_TYPE  *rp;
+   Elf_Ehdr*ehdp;
+   Elf_Phdr*phdp;
 
/*
 * Scan argument and environment vectors. Find dynamic
@@ -189,4 +191,30 @@ _dl_boot_bind(const long sp, long *dl_da
 * we have been fully relocated here, so most things no longer
 * need the loff adjustment
 */
+
+   /*
+* No further changes to the PLT and/or GOT are needed so make
+* them read-only.
+*/
+
+   /* do any RWX -> RX fixups for executable PLTs and apply GNU_RELRO */
+   ehdp = (Elf_Ehdr *)loff;
+   phdp = (Elf_Phdr *)(loff + ehdp->e_phoff);
+   for (i = 0; i < dl_data[AUX_phnum]; i++, phdp++) {
+   switch (phdp->p_type) {
+#if defined(__alpha__) || defined(__hppa__) || defined(__powerpc__) || \
+defined(__sparc64__)
+   case PT_LOAD:
+   if ((phdp->p_flags & (PF_X | PF_W)) != (PF_X | PF_W))
+   break;
+   _dl_mprotect((void *)(phdp->p_vaddr + loff),
+   phdp->p_memsz, PROT_READ);
+   break;
+#endif
+   case PT_GNU_RELRO:
+   _dl_mprotect((void *)(phdp->p_vaddr + loff),
+   phdp->p_memsz, PROT_READ);
+   break;
+   }
+   }
 }
Index: libexec/ld.so/loader.c
===
RCS file: /cvs/src/libexec/ld.so/loader.c,v
retrieving revision 1.167
diff -u -p -r1.167 loader.c
--- libexec/ld.so/loader.c  28 Aug 2016 04:33:17 -  1.167
+++ libexec/ld.so/loader.c  2 Jan 2017 15:55:52 -
@@ -413,25 +413,6 @@ _dl_boot(const char **argv, char **envp,
 #define ROUND_PG(x) (((x) + align) & ~(align))
 #define TRUNC_PG(x) ((x) & ~(align))
 
-   /*
-* now that GOT and PLT have been relocated, and we know
-* page size, protect them from modification
-*/
-#ifndef  RTLD_NO_WXORX
-   {
-   extern char *__got_start;
-   extern char *__got_end;
-
-   if (&__got_start != &__got_end) {
-   _dl_mprotect((void *)ELF_TRUNC((long)&__got_start,
-   _dl_pagesz),
-   ELF_ROUND((long)&__got_end,_dl_pagesz) -
-   ELF_TRUNC((long)&__got_start, _dl_pagesz),
-   GOT_PERMS);
-   }
-   }
-#endif
-
_dl_setup_env(argv[0], envp);
 
DL_DEB(("rtld loading: '%s'\n", __progname));
Index: libexec/ld.so/alpha/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/alpha/archdep.h,v
retrieving revision 1.17
diff -u -p -r1.17 archdep.h
--- libexec/ld.so/alpha/archdep.h   6 Dec 2015 23:36:12 -   1.17
+++ libexec/ld.so/alpha/archdep.h   2 Jan 2017 15:55:52 -
@@ -65,6 +65,4 @@ RELOC_DYN(Elf64_Rela *r, const Elf64_Sym
 
 #define RELOC_GOT(obj, offs)
 
-#define GOT_PERMS PROT_READ
-
 #endif /* _ALPHA_ARCHDEP_H_ */
Index: libexec/ld.so/amd64/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/amd64/archdep.h,v
retrieving revision 1.8
diff -u -p -r1.8 archdep.h
--- libexec/ld.so/amd64/archdep.h   18 May 2016 20:40:20 -  1.8
+++ libexec/ld.so/amd64/archdep.h   2 Jan 2017 15:55:52 -
@@ -68,6 +68,4 @@ RELOC_DYN(Elf64_Rela *r, const Elf64_Sym
 
 #define RELOC_GOT(obj, offs)
 
-#define GOT_PERMS PROT_READ
-
 #endif /* _X86_64_ARCHDEP_H_ */
Index: libexec/ld.so/arm/archdep.h
===
RCS file: /cvs/src/libexec/ld.so/arm/archdep.h,v
retrieving revision 1.8
diff -u -p -r1.8 archdep.h
--- libexec/ld.so/arm/archdep.h 8 Sep 2016 18:56:58 -   1.8
+++ libexec/ld.so/arm/archdep.h 2 Jan 2017 15:55:52 -
@@ -73,6 +73,4 @@ RELOC_DYN(Elf_Rel *r, const Elf_Sym *s, 
 
 #define RELOC_GOT(obj, offs)
 
-#define GOT_PERMS (PROT_READ|PROT_EXEC)
-
 #endif /* _ARM_ARCHDEP_H_ */
Index: libexec/ld.so/hppa/archdep.h

Re: ld.so diff

2012-01-08 Thread Miod Vallat 
 Hi,
 
 Posix says that mmap(2)ing 0 bytes is bad and furthermore, our subsystem
 is not written to support this (because there is no difference between
 no allocation and a 0-byte allocation).
 Strictly speaking, mmap(2) is to return EINVAL for 0 byte allocations
 and I intend to get that into the kernel. But before that can happen,
 ld.so must cease performing 0-byte mmaps. Hence the diff below.
 
 This diff is called mmap0_ld.so.diff.0, has no api/abi change and treats
 a 0-byte area mmap as a noop (i.e. skips the corresponding mmap call).
 
 ok?

Wouldn't it be simpler, in library_mquery.c, to skip LDLIST_INSERT() for
size == 0 elements?

Miod

Re: ld.so diff

2012-01-08 Thread Ariane van der Steldt 
On Sun, Jan 08, 2012 at 01:05:27PM +, Miod Vallat wrote:
  Posix says that mmap(2)ing 0 bytes is bad and furthermore, our subsystem
  is not written to support this (because there is no difference between
  no allocation and a 0-byte allocation).
  Strictly speaking, mmap(2) is to return EINVAL for 0 byte allocations
  and I intend to get that into the kernel. But before that can happen,
  ld.so must cease performing 0-byte mmaps. Hence the diff below.
  
  This diff is called mmap0_ld.so.diff.0, has no api/abi change and treats
  a 0-byte area mmap as a noop (i.e. skips the corresponding mmap call).
  
  ok?
 
 Wouldn't it be simpler, in library_mquery.c, to skip LDLIST_INSERT() for
 size == 0 elements?

You're absolutely right. Updated diff (mmap0_ld.so.diff.1) below.

ok?
-- 
Ariane


Index: libexec/ld.so/library.c
===
RCS file: /cvs/src/libexec/ld.so/library.c,v
retrieving revision 1.63
diff -u -d -p -r1.63 library.c
--- libexec/ld.so/library.c 28 Nov 2011 20:59:03 -  1.63
+++ libexec/ld.so/library.c 8 Jan 2012 05:36:13 -
@@ -181,17 +181,20 @@ _dl_tryload_shlib(const char *libname, i
Elf_Addr size = off + phdp-p_filesz;
void *res;
 
-   res = _dl_mmap(start, ROUND_PG(size),
-   PFLAGS(phdp-p_flags),
-   MAP_FIXED|MAP_PRIVATE, libfile,
-   TRUNC_PG(phdp-p_offset));
+   if (size != 0) {
+   res = _dl_mmap(start, ROUND_PG(size),
+   PFLAGS(phdp-p_flags),
+   MAP_FIXED|MAP_PRIVATE, libfile,
+   TRUNC_PG(phdp-p_offset));
+   } else
+   res = NULL; /* silence gcc */
next_load = _dl_malloc(sizeof(struct load_list));
next_load-next = load_list;
load_list = next_load;
next_load-start = start;
next_load-size = size;
next_load-prot = PFLAGS(phdp-p_flags);
-   if (_dl_mmap_error(res)) {
+   if (size != 0  _dl_mmap_error(res)) {
_dl_printf(%s: rtld mmap failed mapping %s.\n,
_dl_progname, libname);
_dl_close(libfile);
Index: libexec/ld.so/library_mquery.c
===
RCS file: /cvs/src/libexec/ld.so/library_mquery.c,v
retrieving revision 1.39
diff -u -d -p -r1.39 library_mquery.c
--- libexec/ld.so/library_mquery.c  28 Nov 2011 20:59:03 -  1.39
+++ libexec/ld.so/library_mquery.c  8 Jan 2012 16:49:52 -
@@ -157,15 +157,17 @@ _dl_tryload_shlib(const char *libname, i
off = (phdp-p_vaddr  align);
size = off + phdp-p_filesz;
 
-   ld = _dl_malloc(sizeof(struct load_list));
-   ld-start = NULL;
-   ld-size = size;
-   ld-moff = TRUNC_PG(phdp-p_vaddr);
-   ld-foff = TRUNC_PG(phdp-p_offset);
-   ld-prot = PFLAGS(phdp-p_flags);
-   LDLIST_INSERT(ld);
+   if (size != 0) {
+   ld = _dl_malloc(sizeof(struct load_list));
+   ld-start = NULL;
+   ld-size = size;
+   ld-moff = TRUNC_PG(phdp-p_vaddr);
+   ld-foff = TRUNC_PG(phdp-p_offset);
+   ld-prot = PFLAGS(phdp-p_flags);
+   LDLIST_INSERT(ld);
+   }
 
-   if ((ld-prot  PROT_WRITE) == 0 ||
+   if ((PFLAGS(phdp-p_flags)  PROT_WRITE) == 0 ||
ROUND_PG(size) == ROUND_PG(off + phdp-p_memsz))
break;
/* This phdr has a zfod section */

ld.so diff

2012-01-07 Thread Ariane van der Steldt 
Hi,

Posix says that mmap(2)ing 0 bytes is bad and furthermore, our subsystem
is not written to support this (because there is no difference between
no allocation and a 0-byte allocation).
Strictly speaking, mmap(2) is to return EINVAL for 0 byte allocations
and I intend to get that into the kernel. But before that can happen,
ld.so must cease performing 0-byte mmaps. Hence the diff below.

This diff is called mmap0_ld.so.diff.0, has no api/abi change and treats
a 0-byte area mmap as a noop (i.e. skips the corresponding mmap call).

ok?
-- 
Ariane


Index: libexec/ld.so/library.c
===
RCS file: /cvs/src/libexec/ld.so/library.c,v
retrieving revision 1.63
diff -u -d -p -r1.63 library.c
--- libexec/ld.so/library.c 28 Nov 2011 20:59:03 -  1.63
+++ libexec/ld.so/library.c 8 Jan 2012 05:36:13 -
@@ -181,17 +181,20 @@ _dl_tryload_shlib(const char *libname, i
Elf_Addr size = off + phdp-p_filesz;
void *res;
 
-   res = _dl_mmap(start, ROUND_PG(size),
-   PFLAGS(phdp-p_flags),
-   MAP_FIXED|MAP_PRIVATE, libfile,
-   TRUNC_PG(phdp-p_offset));
+   if (size != 0) {
+   res = _dl_mmap(start, ROUND_PG(size),
+   PFLAGS(phdp-p_flags),
+   MAP_FIXED|MAP_PRIVATE, libfile,
+   TRUNC_PG(phdp-p_offset));
+   } else
+   res = NULL; /* silence gcc */
next_load = _dl_malloc(sizeof(struct load_list));
next_load-next = load_list;
load_list = next_load;
next_load-start = start;
next_load-size = size;
next_load-prot = PFLAGS(phdp-p_flags);
-   if (_dl_mmap_error(res)) {
+   if (size != 0  _dl_mmap_error(res)) {
_dl_printf(%s: rtld mmap failed mapping %s.\n,
_dl_progname, libname);
_dl_close(libfile);
Index: libexec/ld.so/library_mquery.c
===
RCS file: /cvs/src/libexec/ld.so/library_mquery.c,v
retrieving revision 1.39
diff -u -d -p -r1.39 library_mquery.c
--- libexec/ld.so/library_mquery.c  28 Nov 2011 20:59:03 -  1.39
+++ libexec/ld.so/library_mquery.c  8 Jan 2012 05:30:25 -
@@ -245,6 +245,9 @@ retry:
off_t foff;
void *res;
 
+   if (ld-size == 0)
+   continue;
+
if (ld-foff  0) {
fd = -1;
foff = 0;