[v5,4/4] elf: Add glibc.rtld.execstack
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
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
* Adhemerval Zanella:
> diff --git a/elf/Makefile b/elf/Makefile
> index bdd169f4e8..6dd15c63d9 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -556,6 +556,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
> ifeq ($(have-depaudit),yes)
> tests += \
> @@ -651,6 +658,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))
Should this be guarded by $(run-built-tests)?
> 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}.
This is ambiguous because 1 enables automatic handling. Executable
stacks will still be disabled in most cases.
> +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
This paragraph seems to have some sort of formatting glitch.
Could we get a couple more modes?
fully disabled, load/dlopen will fail
fully disabled, but load/dlopen will proceed (useful with setarch -X)
auto-enable, dlopen might fail (the default)
auto-enable, dlopen will silently proceed instead of failing
always enabled (useful for legacy applications that need dlopen)
Thanks,
Florian
On 29/11/24 17:01, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index bdd169f4e8..6dd15c63d9 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -556,6 +556,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
>> ifeq ($(have-depaudit),yes)
>> tests += \
>> @@ -651,6 +658,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))
>
> Should this be guarded by $(run-built-tests)?
I don't think it is necessary, since tests-execstack-special-yes will only be non-empty
if $(run-built-tests) is 'yes' (in fact I fixed it from v4, where I saw the tests were
failing on Hurd due the missing $(run-built-tests) check).
>
>> 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}.
>
> This is ambiguous because 1 enables automatic handling. Executable
> stacks will still be disabled in most cases.
I think we can phrase this better:
The @code{glibc.rtld.execstack} tunable allows the user to control how
to proceed regarding the stack execution support. Setting its value to @code{0}
disables executable stacks, even if the program or direct dependencies require
it. The value @code{1} will follow the program and dependencies requirement,
where glibc might turn the initial and subsequent thread stack executable,
if required. 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
>
> This paragraph seems to have some sort of formatting glitch.
Hum, which kind of glitch? On libc.pdf I see that 'NB:' is correctly rendered
on same line of the 'When executable stack...'.
>
> Could we get a couple more modes?
>
> fully disabled, load/dlopen will fail
> fully disabled, but load/dlopen will proceed (useful with setarch -X)
> auto-enable, dlopen might fail (the default)
> auto-enable, dlopen will silently proceed instead of failing
> always enabled (useful for legacy applications that need dlopen)
I am not sure if all these modes do make sense:
1. 'fully disabled, but load/dlopen will proceed (useful with setarch -X)' -
means that either we will need to reinstate the code to make the thread
executable again, or loading a module that requires executable stack
without actually making all the thread stack executable.
2. 'auto-enable, dlopen will silently proceed instead of failing' -
same for 1, it will put the program in an inconsistent wrt stack
execution requirements.
3. 'always enabled (useful for legacy applications that need dlopen)' -
same for 1 wrt reinstate code.
I really don't think it would be good to way to just override the security
hardening this patchset it aiming to fix with a tunable. I think
if a program or any of its dependencies do require executable stack,
it should be well defined during *program loading*, which can be easily
verifiable without the need to run the program.
@@ -38,6 +38,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.
@@ -556,6 +556,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
ifeq ($(have-depaudit),yes)
tests += \
@@ -651,6 +658,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 \
@@ -1923,6 +1931,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
@@ -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
@@ -1284,7 +1285,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;
@@ -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. */
@@ -135,6 +135,12 @@ glibc {
maxval: 1
default: 0
}
+ execstack {
+ type: INT_32
+ minval: 0
+ maxval: 1
+ default: 1
+ }
}
mem {
@@ -1644,6 +1644,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 (GL(dl_rtld_map).l_info[DT_SONAME] != NULL
@@ -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]+)
@@ -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