diff mbox series

[v6,3/3] elf: Add glibc.rtld.execstack

Message ID 20241223144954.3823971-4-adhemerval.zanella@linaro.org
State New
Headers show
Series Improve executable stack handling | expand

Commit Message

Adhemerval Zanella Dec. 23, 2024, 2:49 p.m. UTC 
The new tunable can be used to control whether executable stacks are
allowed from either the main program or dependencies.  The default is
to allow executable stacks.

The executable stacks default permission is checked agains the one
provided by the PT_GNU_STACK from program headers (if present).  The
tunable also disables the stack permission change if any dependency
requires an executable stack at loading time.

Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
 NEWS                           |  5 ++++
 elf/Makefile                   | 44 ++++++++++++++++++++++++++++++++++
 elf/dl-load.c                  |  4 +++-
 elf/dl-support.c               |  5 ++++
 elf/dl-tunables.list           |  6 +++++
 elf/rtld.c                     |  4 ++++
 elf/tst-rtld-list-tunables.exp |  1 +
 manual/tunables.texi           | 19 +++++++++++++++
 8 files changed, 87 insertions(+), 1 deletion(-)

Comments

Florian Weimer Dec. 23, 2024, 5:33 p.m. UTC | #1 
* Adhemerval Zanella:

> diff --git a/manual/tunables.texi b/manual/tunables.texi
> index 0b1b2898c0..c3e894f4fe 100644
> --- a/manual/tunables.texi
> +++ b/manual/tunables.texi
> @@ -355,6 +355,25 @@ tests for @code{AT_SECURE} programs and not meant to be a security feature.
>  The default value of this tunable is @samp{0}.
>  @end deftp
>  
> +@deftp Tunable glibc.rtld.execstack
> +@theglibc{} will use either the default architecture ABI flags (that might
> +contain the executable bit) or the value of @code{PT_GNU_STACK} (if present)
> +to define whether to mark the stack non-executable, and if the program or
> +any shared library dependency require an executable stack the loader will
> +change the main stack permission if kernel starts with a non executable stack.
> +
> +The @code{glibc.rtld.execstack} tunable allows the user to control how
> +to proceed regarding the stack execution bit.  Setting its value to @code{0}
> +disables executable stacks, where @code{1} enables it. The default value
> +is @code{1}.

Isn't this quite misleading?  As far as I understand it, 0 disables
auto-negotiation (not executable stacks, see below), and 1 enables
auto-negotiation.  In both cases, stacks can end up executable or
non-executable from a functionality point of view.  This would perhaps
be cleaner to explain if there was a mode that disables
auto-negotiation, but tries to enable executable stacks if possible at
all; that could help with the dlopen issue mentioned below, too.  (The
NEWS entry is much clearer in this regard.)

> +When executable stacks are not allowed, and if the main program requires an
> +executable stack, the loader will fail with an error message.
> +@strong{NB:} Trying to load a dynamic shared library with @code{dlopen} or
> +@code{dlmopen} that requires an executable stack will always fail if the
> +default flags does not contain the executable bit.
> +@end deftp

Missing paragraph break before @strong{NB:}.  I think this needs to be
rephrased so that it's clear that this is true regardless the setting of
the tunable.  And I assume the gating factor is whether an executable
stack was initially enabled at process startup (for whatever reason),
not what the “default flags” (where?) say.  At least I hope the
implementation behaves this way.

Maybe also add:

“
Some systems do not have separate page protection flags at the hardware
level for read access and execute access (sometimes referred to as
read-implies-exec).  This mode can also be enabled on certain systems
where the hardware supports separate protection flags.  The glibc
tunable configuration is independent of hardware capabilities and kernel
configuration.
”

This doesn't explain that the kernel tracks PROT_EXEC in software even
if the hardware does not enforce it, but this level of detail probably
isn't required here.

Thanks,
Florian
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index f91c4c2421..c0a5dd8ad3 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,11 @@  Major new features:
   liable to change.  Features from C2Y are also enabled by _GNU_SOURCE, or
   by compiling with "gcc -std=gnu2y".
 
+* A new tunable, glibc.rtld.execstack, can be used to control whether
+  executable stacks are allowed from main program, either implicitly due
+  a mising GNU_STACK ELF header or explicit explicitly because of the
+  executable bit in GNU_STACK.  The default is to allow executable stacks.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The big-endian ARC port (arceb-linux-gnu) has been removed.
diff --git a/elf/Makefile b/elf/Makefile
index cea48e9537..4874d9b59e 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -570,6 +570,13 @@  tests-execstack-yes = \
 tests-execstack-static-yes = \
   tst-execstack-prog-static
   # tests-execstack-static-yes
+ifeq (yes,$(run-built-tests))
+tests-execstack-special-yes = \
+  $(objpfx)tst-execstack-needed-noexecstack.out \
+  $(objpfx)tst-execstack-prog-noexecstack.out \
+  $(objpfx)tst-execstack-prog-static-noexecstack.out \
+  # tests-execstack-special-yes
+endif # $(run-built-tests)
 endif
 endif
 ifeq ($(have-depaudit),yes)
@@ -666,6 +673,7 @@  $(objpfx)tst-rtld-dash-dash.out: tst-rtld-dash-dash.sh $(objpfx)ld.so
 
 tests += $(tests-execstack-$(have-z-execstack))
 tests-static+= $(tests-execstack-static-$(have-z-execstack))
+tests-special += $(tests-execstack-special-$(have-z-execstack))
 ifeq ($(run-built-tests),yes)
 tests-special += \
   $(objpfx)tst-ldconfig-X.out \
@@ -1989,6 +1997,42 @@  CFLAGS-tst-execstack-mod.c += -Wno-trampolines
 
 LDFLAGS-tst-execstack-prog-static = -Wl,-z,execstack
 CFLAGS-tst-execstack-prog-static.c += -Wno-trampolines
+
+ifeq (yes,$(build-hardcoded-path-in-tests))
+tst-execstack-prog-noexecstack-msg = "Fatal glibc error: executable stack is not allowed$$"
+else
+tst-execstack-prog-noexecstack-msg = "error while loading shared libraries:.*cannot enable executable stack as shared object requires:"
+endif
+
+$(objpfx)tst-execstack-prog-noexecstack.out: $(objpfx)tst-execstack-prog
+	$(test-program-cmd-before-env) \
+		$(run-program-env) \
+		GLIBC_TUNABLES=glibc.rtld.execstack=0 \
+		$(test-program-cmd-after-env) $< \
+		> $@ 2>&1; echo "status: $$?" >> $@; \
+	grep -q $(tst-execstack-prog-noexecstack-msg) $@ \
+	  && grep -q '^status: 127$$' $@; \
+	  $(evaluate-test)
+
+$(objpfx)tst-execstack-needed-noexecstack.out: $(objpfx)tst-execstack-needed
+	$(test-program-cmd-before-env) \
+		$(run-program-env) \
+		GLIBC_TUNABLES=glibc.rtld.execstack=0 \
+		$(test-program-cmd-after-env) $< \
+		> $@ 2>&1; echo "status: $$?" >> $@; \
+	grep -q 'error while loading shared libraries:.*cannot enable executable stack as shared object requires:' $@ \
+	  && grep -q '^status: 127$$' $@; \
+	  $(evaluate-test)
+
+$(objpfx)tst-execstack-prog-static-noexecstack.out: $(objpfx)tst-execstack-prog-static
+	$(test-program-cmd-before-env) \
+		$(run-program-env) \
+		GLIBC_TUNABLES=glibc.rtld.execstack=0 \
+		$< \
+		> $@ 2>&1; echo "status: $$?" >> $@; \
+	grep -q 'Fatal glibc error: executable stack is not allowed$$' $@ \
+	  && grep -q '^status: 127$$' $@; \
+	  $(evaluate-test)
 endif
 
 LDFLAGS-tst-array2 = -Wl,--no-as-needed
diff --git a/elf/dl-load.c b/elf/dl-load.c
index a238ff4286..76430e26da 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -32,6 +32,7 @@ 
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <gnu/lib-names.h>
+#include <dl-tunables.h>
 
 /* Type for the buffer we put the ELF header and hopefully the program
    header.  This buffer does not really have to be too large.  In most
@@ -1317,7 +1318,8 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
       /* The stack is presently not executable, but this module
 	 requires that it be executable.  Only tries to change the
 	 stack protection during process startup.  */
-      if ((mode & __RTLD_DLOPEN) == 0)
+      if ((mode & __RTLD_DLOPEN) == 0
+	  && TUNABLE_GET (glibc, rtld, execstack, int32_t, NULL) == 1)
 	errval = _dl_make_stack_executable (stack_endp);
       else
 	errval = EINVAL;
diff --git a/elf/dl-support.c b/elf/dl-support.c
index fe1f8c8f6a..73fcd33892 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -45,6 +45,7 @@ 
 #include <dl-find_object.h>
 #include <array_length.h>
 #include <dl-symbol-redir-ifunc.h>
+#include <dl-tunables.h>
 
 extern char *__progname;
 char **_dl_argv = &__progname;	/* This is checked for some error messages.  */
@@ -331,6 +332,10 @@  _dl_non_dynamic_init (void)
 	break;
       }
 
+  if ((__glibc_unlikely (GL(dl_stack_flags)) & PF_X)
+      && TUNABLE_GET (glibc, rtld, execstack, int32_t, NULL) == 0)
+    _dl_fatal_printf ("Fatal glibc error: executable stack is not allowed\n");
+
   call_function_static_weak (_dl_find_object_init);
 
   /* Setup relro on the binary itself.  */
diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index 40ac5b3776..8e656296bb 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -135,6 +135,12 @@  glibc {
       maxval: 1
       default: 0
     }
+    execstack {
+      type: INT_32
+      minval: 0
+      maxval: 1
+      default: 1
+    }
   }
 
   mem {
diff --git a/elf/rtld.c b/elf/rtld.c
index 5eb130be30..8dd0381985 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1645,6 +1645,10 @@  dl_main (const ElfW(Phdr) *phdr,
 
   bool has_interp = rtld_setup_main_map (main_map);
 
+  if ((__glibc_unlikely (GL(dl_stack_flags)) & PF_X)
+      && TUNABLE_GET (glibc, rtld, execstack, int32_t, NULL) == 0)
+    _dl_fatal_printf ("Fatal glibc error: executable stack is not allowed\n");
+
   /* If the current libname is different from the SONAME, add the
      latter as well.  */
   if (_dl_rtld_map.l_info[DT_SONAME] != NULL
diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp
index db0e1c86e9..9f5990f340 100644
--- a/elf/tst-rtld-list-tunables.exp
+++ b/elf/tst-rtld-list-tunables.exp
@@ -13,5 +13,6 @@  glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+)
 glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+)
 glibc.rtld.dynamic_sort: 2 (min: 1, max: 2)
 glibc.rtld.enable_secure: 0 (min: 0, max: 1)
+glibc.rtld.execstack: 1 (min: 0, max: 1)
 glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10)
 glibc.rtld.optional_static_tls: 0x200 (min: 0x0, max: 0x[f]+)
diff --git a/manual/tunables.texi b/manual/tunables.texi
index 0b1b2898c0..c3e894f4fe 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -355,6 +355,25 @@  tests for @code{AT_SECURE} programs and not meant to be a security feature.
 The default value of this tunable is @samp{0}.
 @end deftp
 
+@deftp Tunable glibc.rtld.execstack
+@theglibc{} will use either the default architecture ABI flags (that might
+contain the executable bit) or the value of @code{PT_GNU_STACK} (if present)
+to define whether to mark the stack non-executable, and if the program or
+any shared library dependency require an executable stack the loader will
+change the main stack permission if kernel starts with a non executable stack.
+
+The @code{glibc.rtld.execstack} tunable allows the user to control how
+to proceed regarding the stack execution bit.  Setting its value to @code{0}
+disables executable stacks, where @code{1} enables it. The default value
+is @code{1}.
+
+When executable stacks are not allowed, and if the main program requires an
+executable stack, the loader will fail with an error message.
+@strong{NB:} Trying to load a dynamic shared library with @code{dlopen} or
+@code{dlmopen} that requires an executable stack will always fail if the
+default flags does not contain the executable bit.
+@end deftp
+
 @node Elision Tunables
 @section Elision Tunables
 @cindex elision tunables