[PATCH v2] nptl: Fix MADV_GUARD_INSTALL logic for thread without guard page (BZ 33356)
Adhemerval Zanella
adhemerval.zanella@linaro.org
Mon Sep 8 16:06:13 GMT 2025
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;
--
2.43.0
More information about the Libc-alpha
mailing list