[PATCH v2] nptl: Fix MADV_GUARD_INSTALL logic for thread without guard page (BZ 33356)
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Sep 15 18:43:30 GMT 2025
Ping.
On 08/09/25 13:06, Adhemerval Zanella wrote:
> The main issue is that setup_stack_prot fails to account for cases where
> the cached thread stack lacks a guard page, which can cause madvise to
> fail. Update the logic to also handle whether MADV_GUARD_INSTALL is
> supported when resizing the guard page.
>
> Checked on x86_64-linux-gnu with 6.8.0 and 6.15 kernels.
> --
> Changes from v1:
> * Fixed typos.
> * Improve tst-guard1.
> ---
> nptl/allocatestack.c | 22 +++++++++++++----
> nptl/tst-guard1.c | 59 +++++++++++++++++++++++++++++---------------
> 2 files changed, 56 insertions(+), 25 deletions(-)
>
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 4c2aacdd10..99f56b94df 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -234,7 +234,7 @@ setup_stack_prot (char *mem, size_t size, struct pthread *pd,
> /* Update the guard area of the thread stack MEM of size SIZE with the new
> GUARDISZE. It uses the method defined by PD stack_mode. */
> static inline bool
> -adjust_stack_prot (char *mem, size_t size, const struct pthread *pd,
> +adjust_stack_prot (char *mem, size_t size, struct pthread *pd,
> size_t guardsize, size_t pagesize_m1)
> {
> /* The required guard area is larger than the current one. For
> @@ -252,11 +252,23 @@ adjust_stack_prot (char *mem, size_t size, const struct pthread *pd,
> so use the new guard placement with the new size. */
> if (guardsize > pd->guardsize)
> {
> + /* There was no need to previously setup a guard page, so we need
> + to check whether the kernel supports guard advise. */
> char *guard = guard_position (mem, size, guardsize, pd, pagesize_m1);
> - if (pd->stack_mode == ALLOCATE_GUARD_MADV_GUARD)
> - return __madvise (guard, guardsize, MADV_GUARD_INSTALL) == 0;
> - else if (pd->stack_mode == ALLOCATE_GUARD_PROT_NONE)
> - return __mprotect (guard, guardsize, PROT_NONE) == 0;
> + if (atomic_load_relaxed (&allocate_stack_mode)
> + == ALLOCATE_GUARD_MADV_GUARD)
> + {
> + if (__madvise (guard, guardsize, MADV_GUARD_INSTALL) == 0)
> + {
> + pd->stack_mode = ALLOCATE_GUARD_MADV_GUARD;
> + return true;
> + }
> + atomic_store_relaxed (&allocate_stack_mode,
> + ALLOCATE_GUARD_PROT_NONE);
> + }
> +
> + pd->stack_mode = ALLOCATE_GUARD_PROT_NONE;
> + return __mprotect (guard, guardsize, PROT_NONE) == 0;
> }
> /* The current guard area is larger than the required one. For
> _STACK_GROWS_DOWN is means change the guard as:
> diff --git a/nptl/tst-guard1.c b/nptl/tst-guard1.c
> index e3e06df0fc..027c282a04 100644
> --- a/nptl/tst-guard1.c
> +++ b/nptl/tst-guard1.c
> @@ -21,6 +21,7 @@
> #include <setjmp.h>
> #include <stackinfo.h>
> #include <stdio.h>
> +#include <support/capture_subprocess.h>
> #include <support/check.h>
> #include <support/test-driver.h>
> #include <support/xsignal.h>
> @@ -202,7 +203,7 @@ tf (void *closure)
>
> /* Test 1: caller provided stack without guard. */
> static void
> -do_test1 (void)
> +do_test1 (void *closure)
> {
> pthread_attr_t attr;
> xpthread_attr_init (&attr);
> @@ -227,7 +228,7 @@ do_test1 (void)
>
> /* Test 2: same as 1., but with a guard area. */
> static void
> -do_test2 (void)
> +do_test2 (void *closure)
> {
> pthread_attr_t attr;
> xpthread_attr_init (&attr);
> @@ -250,18 +251,9 @@ do_test2 (void)
> xmunmap (stack, stacksize);
> }
>
> -/* Test 3: pthread_create with default values. */
> +/* Test 3: pthread_create without a guard area. */
> static void
> -do_test3 (void)
> -{
> - pthread_t t = xpthread_create (NULL, tf, NULL);
> - void *status = xpthread_join (t);
> - TEST_VERIFY (status == 0);
> -}
> -
> -/* Test 4: pthread_create without a guard area. */
> -static void
> -do_test4 (void)
> +do_test3 (void *closure)
> {
> pthread_attr_t attr;
> xpthread_attr_init (&attr);
> @@ -277,9 +269,18 @@ do_test4 (void)
> xpthread_attr_destroy (&attr);
> }
>
> +/* Test 4: pthread_create with default values. */
> +static void
> +do_test4 (void *closure)
> +{
> + pthread_t t = xpthread_create (NULL, tf, NULL);
> + void *status = xpthread_join (t);
> + TEST_VERIFY (status == 0);
> +}
> +
> /* Test 5: pthread_create with non default stack and guard size value. */
> static void
> -do_test5 (void)
> +do_test5 (void *closure)
> {
> pthread_attr_t attr;
> xpthread_attr_init (&attr);
> @@ -299,7 +300,7 @@ do_test5 (void)
> test 3, but with a larger guard area. The pthread_create will need to
> increase the guard area. */
> static void
> -do_test6 (void)
> +do_test6 (void *closure)
> {
> pthread_attr_t attr;
> xpthread_attr_init (&attr);
> @@ -320,7 +321,7 @@ do_test6 (void)
> pthread_create should use the cached stack from previous tests, but it
> would require to reduce the guard area. */
> static void
> -do_test7 (void)
> +do_test7 (void *closure)
> {
> pthread_t t = xpthread_create (NULL, tf, NULL);
> void *status = xpthread_join (t);
> @@ -346,21 +347,39 @@ do_test (void)
>
> static const struct {
> const char *descr;
> - void (*test)(void);
> + void (*test)(void *);
> } tests[] = {
> { "user provided stack without guard", do_test1 },
> { "user provided stack with guard", do_test2 },
> - { "default attribute", do_test3 },
> - { "default attribute without guard", do_test4 },
> + /* N.B: do_test3 should be before do_test4 to check if a new thread
> + that uses the thread stack previously allocated without a guard
> + page correctly sets up the guard pages even on a kernel without
> + MADV_GUARD_INSTALL support (BZ 33356). */
> + { "default attribute without guard", do_test3 },
> + { "default attribute", do_test4 },
> + /* Also checks if the guard is correctly removed from the cache thread
> + stack. */
> + { "default attribute without guard", do_test3 },
> { "non default stack and guard sizes", do_test5 },
> { "reused stack with larger guard", do_test6 },
> { "reused stack with smaller guard", do_test7 },
> };
>
> + /* Run each test with a clean state. */
> + for (int i = 0; i < array_length (tests); i++)
> + {
> + printf ("debug: fork: test%01d: %s\n", i, tests[i].descr);
> + struct support_capture_subprocess result =
> + support_capture_subprocess (tests[i].test, NULL);
> + support_capture_subprocess_check (&result, tests[i].descr, 0, sc_allow_none);
> + support_capture_subprocess_free (&result);
> + }
> +
> + /* And now run the same tests along with the thread stack cache. */
> for (int i = 0; i < array_length (tests); i++)
> {
> printf ("debug: test%01d: %s\n", i, tests[i].descr);
> - tests[i].test();
> + tests[i].test ( NULL);
> }
>
> return 0;
More information about the Libc-alpha
mailing list