[PATCH] nptl: Fix pthread_getattr_np when modules with execstack are allowed (BZ 32897)

[Luke Drummond]

On Wed Apr 23, 2025 at 7:21 PM BST, Adhemerval Zanella wrote:

> The BZ 32653 fix (12a497c716f0a06be5946cabb8c3ec22a079771e) kept the
> stack pointer zeroing from make_main_stack_executable on
> _dl_make_stack_executable. However, previously the 'stack_endp'
> pointed to temporary variable created before the call of
> _dl_map_object_from_fd; while now we use the __libc_stack_end
> directly.
>
> Since pthread_getattr_np relies on correct __libc_stack_end, if
> _dl_make_stack_executable s called after process startup (for
> instance, when glibc.rtld.execstack=2 is set and a module with an
> executable stack is loaded) __libc_stack_end will be set to zero,
> and the call will always fail.

I found out during our investigations that the loaded library doesn't actually
need to have +X for this to reproduce; only the tunable needs to be set.

>
> The __libc_stack_end zero was used a mitigation hardening, but since
> 52a01100ad011293197637e42b5be1a479a2f4ae it is used solely on
> pthread_getattr_np code. So there is no point in zeroing anymore.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.

Otherwise the patch and tests look good and confirmed working locally on amd64
Linux

All the Best

Luke
> ---
> elf/dl-execstack-tunable.c | 2 +-
> elf/dl-execstack.c | 2 +-
> sysdeps/generic/ldsodefs.h | 2 +-
> sysdeps/mach/hurd/dl-execstack.c | 5 ++--
> sysdeps/pthread/Makefile | 9 ++++++
> sysdeps/pthread/tst-stack2-mod.c | 39 +++++++++++++++++++++++++
> sysdeps/pthread/tst-stack2.c | 40 ++++++++++++++++++++++++++
> sysdeps/unix/sysv/linux/dl-execstack.c | 7 ++---
> 8 files changed, 95 insertions(+), 11 deletions(-)
> create mode 100644 sysdeps/pthread/tst-stack2-mod.c
> create mode 100644 sysdeps/pthread/tst-stack2.c
>
> diff --git a/elf/dl-execstack-tunable.c b/elf/dl-execstack-tunable.c
> index 6cef1a3036..e3b638aeaa 100644
> --- a/elf/dl-execstack-tunable.c
> +++ b/elf/dl-execstack-tunable.c
> @@ -31,7 +31,7 @@ _dl_handle_execstack_tunable (void)
> break;
>  
> case stack_tunable_mode_force:
> - if (_dl_make_stack_executable (&__libc_stack_end) != 0)
> + if (_dl_make_stack_executable (__libc_stack_end) != 0)
> _dl_fatal_printf (
> "Fatal glibc error: cannot enable executable stack as tunable
> requires");
> break;
> diff --git a/elf/dl-execstack.c b/elf/dl-execstack.c
> index e4d7dbe7f8..ceec5b2def 100644
> --- a/elf/dl-execstack.c
> +++ b/elf/dl-execstack.c
> @@ -23,7 +23,7 @@
> so as to mprotect it. */
>  
> int
> -_dl_make_stack_executable (void **stack_endp)
> +_dl_make_stack_executable (const void *stack_endp)
> {
> return ENOSYS;
> }
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index b5d5b3106c..fc4a3de767 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -733,7 +733,7 @@ void _dl_handle_execstack_tunable (void)
> attribute_hidden;
> /* This function changes the permission of the memory region pointed
> by STACK_ENDP to executable and update the internal memory protection
> flags for future thread stack creation. */
> -int _dl_make_stack_executable (void **stack_endp) attribute_hidden;
> +int _dl_make_stack_executable (const void *stack_endp)
> attribute_hidden;
>  
> /* 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
> diff --git a/sysdeps/mach/hurd/dl-execstack.c
> b/sysdeps/mach/hurd/dl-execstack.c
> index 0617d3a161..dc4719bd38 100644
> --- a/sysdeps/mach/hurd/dl-execstack.c
> +++ b/sysdeps/mach/hurd/dl-execstack.c
> @@ -26,12 +26,11 @@ extern struct hurd_startup_data *_dl_hurd_data
> attribute_hidden;
> so as to mprotect it. */
>  
> int
> -_dl_make_stack_executable (void **stack_endp)
> +_dl_make_stack_executable (const void *stack_endp)
> {
> /* Challenge the caller. */
> - if (__builtin_expect (*stack_endp != __libc_stack_end, 0))
> + if (__glibc_unlikely (stack_endp != __libc_stack_end))
> return EPERM;
> - *stack_endp = NULL;
>  
> #if IS_IN (rtld)
> if (__mprotect ((void *)_dl_hurd_data->stack_base,
> _dl_hurd_data->stack_size,
> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> index d4869c624b..5acf505a90 100644
> --- a/sysdeps/pthread/Makefile
> +++ b/sysdeps/pthread/Makefile
> @@ -273,6 +273,7 @@ tests += \
> tst-spin4 \
> tst-spin5 \
> tst-stack1 \
> + tst-stack2 \
> tst-stdio1 \
> tst-stdio2 \
> tst-thrd-detach \
> @@ -368,6 +369,7 @@ modules-names += \
> tst-atfork4mod \
> tst-create1mod \
> tst-fini1mod \
> + tst-stack2-mod \
> tst-tls4moda \
> tst-tls4modb \
> # modules-names
> @@ -541,4 +543,11 @@ LDFLAGS-tst-create1 = -Wl,-export-dynamic
> $(objpfx)tst-create1: $(shared-thread-library)
> $(objpfx)tst-create1.out: $(objpfx)tst-create1mod.so
>  
> +$(objpfx)tst-stack2.out: $(objpfx)tst-stack2-mod.so
> +LDFLAGS-tst-stack2-mod.so = -Wl,-z,execstack
> +ifeq ($(have-no-error-execstack),yes)
> +LDFLAGS-tst-stack2-mod.so += -Wl,--no-error-execstack
> +endif
> +tst-stack2-ENV = GLIBC_TUNABLES=glibc.rtld.execstack=2
> +
> endif
> diff --git a/sysdeps/pthread/tst-stack2-mod.c
> b/sysdeps/pthread/tst-stack2-mod.c
> new file mode 100644
> index 0000000000..806fdbcd8d
> --- /dev/null
> +++ b/sysdeps/pthread/tst-stack2-mod.c
> @@ -0,0 +1,39 @@
> +/* Check if pthread_getattr_np works within modules with non-exectuble
> + stacks (BZ 32897).
> + Copyright (C) 2025 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <pthread.h>
> +
> +bool init_result;
> +
> +void
> +__attribute__ ((constructor))
> +init (void)
> +{
> + pthread_t me = pthread_self ();
> + pthread_attr_t attr;
> + init_result = pthread_getattr_np (me, &attr) == 0;
> +}
> +
> +int
> +mod_func (void)
> +{
> + pthread_t me = pthread_self ();
> + pthread_attr_t attr;
> + return pthread_getattr_np (me, &attr);
> +}
> diff --git a/sysdeps/pthread/tst-stack2.c b/sysdeps/pthread/tst-stack2.c
> new file mode 100644
> index 0000000000..f800390167
> --- /dev/null
> +++ b/sysdeps/pthread/tst-stack2.c
> @@ -0,0 +1,40 @@
> +/* Check if pthread_getattr_np works within modules with non-exectuble
> + stacks (BZ 32897).
> + Copyright (C) 2025 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <stdbool.h>
> +#include <support/xdlfcn.h>
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> + void *h = xdlopen ("tst-stack2-mod.so", RTLD_NOW);
> +
> + bool *init_result = xdlsym (h, "init_result");
> + TEST_COMPARE (*init_result, true);
> +
> + int (*mod_func)(void) = xdlsym (h, "mod_func");
> + TEST_COMPARE (mod_func (), 0);
> +
> + xdlclose (h);
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/dl-execstack.c
> b/sysdeps/unix/sysv/linux/dl-execstack.c
> index 9791b339ca..6db9601656 100644
> --- a/sysdeps/unix/sysv/linux/dl-execstack.c
> +++ b/sysdeps/unix/sysv/linux/dl-execstack.c
> @@ -19,10 +19,10 @@
> #include <ldsodefs.h>
>  
> int
> -_dl_make_stack_executable (void **stack_endp)
> +_dl_make_stack_executable (const void *stack_endp)
> {
> /* This gives us the highest/lowest page that needs to be changed. */
> - uintptr_t page = ((uintptr_t) *stack_endp
> + uintptr_t page = ((uintptr_t) stack_endp
> & -(intptr_t) GLRO(dl_pagesize));
>  
> if (__mprotect ((void *) page, GLRO(dl_pagesize),
> @@ -35,9 +35,6 @@ _dl_make_stack_executable (void **stack_endp)
> ) != 0)
> return errno;
>  
> - /* Clear the address. */
> - *stack_endp = NULL;
> -
> /* Remember that we changed the permission. */
> GL(dl_stack_flags) |= PF_X;
>  
> --
> 2.43.0