Message ID | 20241223144954.3823971-4-adhemerval.zanella@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | Improve executable stack handling | expand |
* 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
On 23/12/24 14:33, Florian Weimer wrote: > * 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.) Ack, I will use the NEWS entry as a based for this part. > >> +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. Ack, and it does 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. > ” Ack. > > 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 --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