This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] nptl: change default stack guard size of threads


RFC patch based on discussions about stack probing at
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02306.html
>From 84da61fd1e0d80d8bfc4c1e867fb5df3b5148caa Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Tue, 28 Nov 2017 11:49:24 +0000
Subject: [PATCH] nptl: change default stack guard size of threads

There are several compiler implementations that allow large stack
allocations to jump over the guard page at the end of the stack and
corrupt memory beyond that. See CVE-2017-1000364.

The linux kernel increased the default guard size of the main thread
to 1M to mitigate the issue, which fixes some of the known local
privilege escalation exploits using setuid binaries, but does not
affect memory corruption in case of thread stack overflow in
multi-threaded processes.

Compilers can emit code to probe the stack such that a guard page
cannot be skipped, but this is not yet available on many targets and
may introduce larger performance overhead than necessary when the
compiler has to assume the minimum supported page size of the target.

On aarch64 the plan is to assume 64K guard size in the probing code
since this makes the overhead sufficiently small that it can be
turned on by default, but 4K page size is also supported, so the
mitigation only works if the libc guard size is increased.

This patch increases the default guard size to 64K which should
prevent jumping over the guard page in most existing binaries, in
particular larger stack allocation should not really exist in glibc
itself because this is the __MAX_ALLOCA_CUTOFF limit.

Note that POSIX 2008 allows implementation defined default guardsize,
but previous versions required it to be one page.

This patch can be controversial because it is not conform to old
standards, changes the behaviour of pthread_attr_init with respect
to the non-standard and underspecified pthread_setattr_default_np,
it increases the address space usage of existing code and because
security of code using smaller guardsize is not addressed. The
address space usage can be a concern on 32bit targets. The change
can be made for aarch64 only but I think it may be useful on other
targets too.

	nptl/descr.h (ARCH_GUARD_DEFAULT_SIZE): Define.
	nptl/nptl-init.c (__pthread_initialize_minimal_internal): Change
	__default_pthread_attr.guardsize.
	nptl/pthread_create.c (__pthread_create_2_0): Use
	__default_pthread_attr.guardsize.
	nptl/pthread_attr_init.c (__pthread_attr_init_2_1): Likewise.
	nptl/tst-attr2.c (do_test): Don't check default guardsize.
---
 nptl/descr.h             | 5 +++++
 nptl/nptl-init.c         | 3 ++-
 nptl/pthread_attr_init.c | 5 +++--
 nptl/pthread_create.c    | 6 ++++--
 nptl/tst-attr2.c         | 6 ------
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/nptl/descr.h b/nptl/descr.h
index c83b17b674..abb86e917a 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -39,6 +39,11 @@
 # define TCB_ALIGNMENT	sizeof (double)
 #endif
 
+/* Default guard size will get rounded up to page size.
+   Targets can override the setting in pthreaddef.h.  */
+#ifndef ARCH_GUARD_DEFAULT_SIZE
+# define ARCH_GUARD_DEFAULT_SIZE 65536
+#endif
 
 /* We keep thread specific data in a special data structure, a two-level
    array.  The top-level array contains pointers to dynamically allocated
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 869e926f17..447913e9ec 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -436,7 +436,8 @@ __pthread_initialize_minimal_internal (void)
   limit.rlim_cur = ALIGN_UP (limit.rlim_cur, pagesz);
   lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
   __default_pthread_attr.stacksize = limit.rlim_cur;
-  __default_pthread_attr.guardsize = GLRO (dl_pagesize);
+  __default_pthread_attr.guardsize = pagesz > ARCH_GUARD_DEFAULT_SIZE
+				     ? pagesz : ARCH_GUARD_DEFAULT_SIZE;
   lll_unlock (__default_pthread_attr_lock, LLL_PRIVATE);
 
 #ifdef SHARED
diff --git a/nptl/pthread_attr_init.c b/nptl/pthread_attr_init.c
index eceaf85dbf..ec063a01ae 100644
--- a/nptl/pthread_attr_init.c
+++ b/nptl/pthread_attr_init.c
@@ -43,8 +43,9 @@ __pthread_attr_init_2_1 (pthread_attr_t *attr)
 
   iattr = (struct pthread_attr *) attr;
 
-  /* Default guard size specified by the standard.  */
-  iattr->guardsize = __getpagesize ();
+  lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
+  iattr->guardsize = __default_pthread_attr.guardsize;
+  lll_unlock (__default_pthread_attr_lock, LLL_PRIVATE);
 
   return 0;
 }
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 51ae60dfca..24fdad6271 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -869,7 +869,9 @@ __pthread_create_2_0 (pthread_t *newthread, const pthread_attr_t *attr,
   if (attr != NULL)
     {
       struct pthread_attr *iattr = (struct pthread_attr *) attr;
-      size_t ps = __getpagesize ();
+      lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
+      size_t guardsize = __default_pthread_attr.guardsize;
+      lll_unlock (__default_pthread_attr_lock, LLL_PRIVATE);
 
       /* Copy values from the user-provided attributes.  */
       new_attr.schedparam = iattr->schedparam;
@@ -878,7 +880,7 @@ __pthread_create_2_0 (pthread_t *newthread, const pthread_attr_t *attr,
 
       /* Fill in default values for the fields not present in the old
 	 implementation.  */
-      new_attr.guardsize = ps;
+      new_attr.guardsize = guardsize;
       new_attr.stackaddr = NULL;
       new_attr.stacksize = 0;
       new_attr.cpuset = NULL;
diff --git a/nptl/tst-attr2.c b/nptl/tst-attr2.c
index 9967b69773..a965c3ac1c 100644
--- a/nptl/tst-attr2.c
+++ b/nptl/tst-attr2.c
@@ -90,12 +90,6 @@ default detach state wrong: %d, expected %d (PTHREAD_CREATE_JOINABLE)\n",
       puts ("1st attr_getguardsize failed");
       exit (1);
     }
-  if (g != (size_t) sysconf (_SC_PAGESIZE))
-    {
-      printf ("default guardsize %zu, expected %ld (PAGESIZE)\n",
-	      g, sysconf (_SC_PAGESIZE));
-      exit (1);
-    }
 
   e = pthread_attr_setguardsize (&a, 0);
   if (e != 0)
-- 
2.11.0


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]