[PATCH] nplt: Fix MADV_GUARD_INSTALL logic for thread without guard page (BZ 33356)

[Adhemerval Zanella]

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.
---
 nptl/allocatestack.c | 22 +++++++++++++++++-----
 nptl/tst-guard1.c    | 31 +++++++++++++++++++------------
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 4c2aacdd10..61838eed8a 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 parge, so we need
+	 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..2c597a6a34 100644
--- a/nptl/tst-guard1.c
+++ b/nptl/tst-guard1.c
@@ -250,18 +250,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)
 {
   pthread_attr_t attr;
   xpthread_attr_init (&attr);
@@ -277,6 +268,15 @@ do_test4 (void)
   xpthread_attr_destroy (&attr);
 }
 
+/* Test 4: pthread_create with default values.  */
+static void
+do_test4 (void)
+{
+  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)
@@ -350,8 +350,15 @@ do_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 setup the guard pages even on 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 },
-- 
2.43.0