Patchwork [1/3,ELF] Allow the machine to override stack permissions via USE_DL_EXEC_STACK_OVERRIDE.

login
register
mail settings
Submitter Dragan Mladjenovic
Date June 27, 2019, 9:50 p.m.
Message ID <1561672142-5907-2-git-send-email-dmladjenovic@wavecomp.com>
Download mbox | patch
Permalink /patch/33466/
State New
Headers show

Comments

Dragan Mladjenovic - June 27, 2019, 9:50 p.m.
This patch allows the machine-dependent code to override non-executable stack
permissions by defining USE_DL_EXEC_STACK_OVERRIDE and implementing _dl_exec_stack_override.
It is called early during the static startup after the os version check and during the load
of every shared library or main executable if ld.so is invoked explicitly.

	* elf/dl-load.c (_dl_map_object_from_fd): Call '_dl_exec_stack_override'.
	* elf/dl-support.c (_dl_non_dynamic_init): Likewise.
	* sysdeps/generic/ldsodefs.h (_dl_exec_stack_override): New prototype.
---
 elf/dl-load.c              | 10 ++++++++++
 elf/dl-support.c           |  8 +++++++-
 sysdeps/generic/ldsodefs.h |  4 ++++
 3 files changed, 21 insertions(+), 1 deletion(-)

-- 
1.9.1
Joseph Myers - July 8, 2019, 11:33 a.m.
On Thu, 27 Jun 2019, Dragan Mladjenovic wrote:

> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 5abeb86..9155b74 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1242,6 +1242,16 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>      /* Adjust the PT_PHDR value by the runtime load address.  */
>      l->l_phdr = (ElfW(Phdr) *) ((ElfW(Addr)) l->l_phdr + l->l_addr);
>  
> +#ifdef USE_DL_EXEC_STACK_OVERRIDE
> +  /* Program requests a non-executable stack, but architecture does
> +     not support it.  */
> +  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
> +    {
> +      errstring = N_("cannot override stack memory protections");
> +      goto call_lose_errno;
> +    }
> +#endif

This sort of #ifdef is not proper glibc style.  You should have a default 
trivial (inline?) definition of _dl_exec_stack_override and then have MIPS 
override the file with that function definition (without duplicating any 
architecture-independent code in the process).  If you have a default 
inline function definition, that means all this code gets checked for 
syntax when building for any architecture, not just for MIPS.
Florian Weimer - July 8, 2019, 11:50 a.m.
* Dragan Mladjenovic:

> +#ifdef USE_DL_EXEC_STACK_OVERRIDE
> +  /* Program requests a non-executable stack, but architecture does
> +     not support it.  */
> +  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))
> +    {
> +      errstring = N_("cannot override stack memory protections");
> +      goto call_lose_errno;
> +    }
> +#endif

Is the comment really correct?  I think the proposed MIPS implementation
is not architecture-specific, but specific to the kernel version.

Thanks,
Florian

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c

index 5abeb86..9155b74 100644

--- a/elf/dl-load.c

+++ b/elf/dl-load.c

@@ -1242,6 +1242,16 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,

     /* Adjust the PT_PHDR value by the runtime load address.  */
     l->l_phdr = (ElfW(Phdr) *) ((ElfW(Addr)) l->l_phdr + l->l_addr);
 
+#ifdef USE_DL_EXEC_STACK_OVERRIDE

+  /* Program requests a non-executable stack, but architecture does

+     not support it.  */

+  if (__glibc_unlikely (_dl_exec_stack_override (&stack_flags) != 0))

+    {

+      errstring = N_("cannot override stack memory protections");

+      goto call_lose_errno;

+    }

+#endif

+

   if (__glibc_unlikely ((stack_flags &~ GL(dl_stack_flags)) & PF_X))
     {
       /* The stack is presently not executable, but this module
diff --git a/elf/dl-support.c b/elf/dl-support.c

index 0a8b636..dd99d58 100644

--- a/elf/dl-support.c

+++ b/elf/dl-support.c

@@ -179,7 +179,6 @@  ElfW(Word) _dl_stack_flags = DEFAULT_STACK_PERMS;

    It returns an errno code or zero on success.  */
 int (*_dl_make_stack_executable_hook) (void **) = _dl_make_stack_executable;
 
-

 /* Function in libpthread to wait for termination of lookups.  */
 void (*_dl_wait_lookup_done) (void);
 
@@ -375,6 +374,13 @@  _dl_non_dynamic_init (void)

 	  _dl_stack_flags = _dl_phdr[i].p_flags;
 	  break;
 	}
+

+#ifdef USE_DL_EXEC_STACK_OVERRIDE

+  if (__glibc_unlikely (_dl_exec_stack_override (&_dl_stack_flags) != 0))

+    {

+      _dl_fatal_printf ("cannot override stack memory protections\n");

+    }

+#endif

 }
 
 #ifdef DL_SYSINFO_IMPLEMENTATION
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h

index b1fc5c3..4e1f0f1 100644

--- a/sysdeps/generic/ldsodefs.h

+++ b/sysdeps/generic/ldsodefs.h

@@ -642,6 +642,10 @@  extern size_t _dl_phnum;

 extern int _dl_make_stack_executable (void **stack_endp);
 rtld_hidden_proto (_dl_make_stack_executable)
 
+#ifdef USE_DL_EXEC_STACK_OVERRIDE

+extern int _dl_exec_stack_override (void *);

+rtld_hidden_proto (_dl_exec_stack_override)

+#endif

 /* Variable pointing to the end of the stack (or close to it).  This value
    must be constant over the runtime of the application.  Some programs
    might use the variable which results in copy relocations on some