diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2024-11-28 14:36:43 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2024-12-31 09:04:20 -0300 |
commit | 0ca8785a28515291d4ef074b5b6cfb27434c1d2b (patch) | |
tree | ba0b23b8b1bcef4d2717f1d605d32ef11518430c /sysdeps/unix | |
parent | ca96ea06b37c8601dcc9579dd4c8619322ab1ea1 (diff) | |
download | glibc-0ca8785a28515291d4ef074b5b6cfb27434c1d2b.zip glibc-0ca8785a28515291d4ef074b5b6cfb27434c1d2b.tar.gz glibc-0ca8785a28515291d4ef074b5b6cfb27434c1d2b.tar.bz2 |
elf: Do not change stack permission on dlopen/dlmopen
If some shared library loaded with dlopen/dlmopen requires an executable
stack, either implicitly because of a missing GNU_STACK ELF header
(where the ABI default flags implies in the executable bit) or explicitly
because of the executable bit from GNU_STACK; the loader will try to set
the both the main thread and all thread stacks (from the pthread cache)
as executable.
Besides the issue where any __nptl_change_stack_perm failure does not
undo the previous executable transition (meaning that if the library
fails to load, there can be thread stacks with executable stacks), this
behavior was used on a CVE [1] as a vector for RCE.
This patch changes that if a shared library requires an executable
stack, and the current stack is not executable, dlopen fails. The
change is done only for dynamically loaded modules, if the program
or any dependency requires an executable stack, the loader will still
change the main thread before program execution and any thread created
with default stack configuration.
[1] https://www.qualys.com/2023/07/19/cve-2023-38408/rce-openssh-forwarded-ssh-agent.txt
Checked on x86_64-linux-gnu and i686-linux-gnu.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Diffstat (limited to 'sysdeps/unix')
-rw-r--r-- | sysdeps/unix/sysv/linux/Versions | 3 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/dl-execstack.c | 67 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/mips/Makefile | 7 |
3 files changed, 9 insertions, 68 deletions
diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions index 213ff5f..55d5655 100644 --- a/sysdeps/unix/sysv/linux/Versions +++ b/sysdeps/unix/sysv/linux/Versions @@ -360,7 +360,4 @@ ld { __rseq_offset; __rseq_size; } - GLIBC_PRIVATE { - __nptl_change_stack_perm; - } } diff --git a/sysdeps/unix/sysv/linux/dl-execstack.c b/sysdeps/unix/sysv/linux/dl-execstack.c index b986898..68db673 100644 --- a/sysdeps/unix/sysv/linux/dl-execstack.c +++ b/sysdeps/unix/sysv/linux/dl-execstack.c @@ -16,19 +16,10 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ -#include <errno.h> #include <ldsodefs.h> -#include <libintl.h> -#include <list.h> -#include <pthreadP.h> -#include <stackinfo.h> -#include <stdbool.h> -#include <sys/mman.h> -#include <sysdep.h> -#include <unistd.h> -static int -make_main_stack_executable (void **stack_endp) +int +_dl_make_stack_executable (void **stack_endp) { /* This gives us the highest/lowest page that needs to be changed. */ uintptr_t page = ((uintptr_t) *stack_endp @@ -52,57 +43,3 @@ make_main_stack_executable (void **stack_endp) return 0; } - -int -_dl_make_stacks_executable (void **stack_endp) -{ - /* First the main thread's stack. */ - int err = make_main_stack_executable (stack_endp); - if (err != 0) - return err; - - lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE); - - list_t *runp; - list_for_each (runp, &GL (dl_stack_used)) - { - err = __nptl_change_stack_perm (list_entry (runp, struct pthread, list)); - if (err != 0) - break; - } - - /* Also change the permission for the currently unused stacks. This - might be wasted time but better spend it here than adding a check - in the fast path. */ - if (err == 0) - list_for_each (runp, &GL (dl_stack_cache)) - { - err = __nptl_change_stack_perm (list_entry (runp, struct pthread, - list)); - if (err != 0) - break; - } - - lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE); - - return err; -} - -int -__nptl_change_stack_perm (struct pthread *pd) -{ -#if _STACK_GROWS_DOWN - void *stack = pd->stackblock + pd->guardsize; - size_t len = pd->stackblock_size - pd->guardsize; -#elif _STACK_GROWS_UP - void *stack = pd->stackblock; - size_t len = (uintptr_t) pd - pd->guardsize - (uintptr_t) pd->stackblock; -#else -# error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP" -#endif - if (__mprotect (stack, len, PROT_READ | PROT_WRITE | PROT_EXEC) != 0) - return errno; - - return 0; -} -rtld_hidden_def (__nptl_change_stack_perm) diff --git a/sysdeps/unix/sysv/linux/mips/Makefile b/sysdeps/unix/sysv/linux/mips/Makefile index d5725c6..05ec915 100644 --- a/sysdeps/unix/sysv/linux/mips/Makefile +++ b/sysdeps/unix/sysv/linux/mips/Makefile @@ -61,6 +61,7 @@ ifeq ($(subdir),elf) # this test is expected to fail. ifneq ($(mips-has-gnustack),yes) test-xfail-check-execstack = yes +CFLAGS-tst-execstack.c += -DDEFAULT_RWX_STACK=1 endif endif @@ -68,6 +69,12 @@ ifeq ($(subdir),stdlib) gen-as-const-headers += ucontext_i.sym endif +ifeq ($(subdir),nptl) +ifeq ($(mips-force-execstack),yes) +CFLAGS-tst-execstack-threads.c += -DDEFAULT_RWX_STACK=1 +endif +endif + ifeq ($(mips-force-execstack),yes) CFLAGS-.o += -Wa,-execstack CFLAGS-.os += -Wa,-execstack |