diff mbox

[BZ,#22637] Fix stack guard size accounting

Message ID 20180115150839.4EA504034FFAF@oldenburg.str.redhat.com
State Not applicable
Headers

Commit Message

Florian Weimer Jan. 15, 2018, 3:08 p.m. UTC 
  From: Szabolcs Nagy <szabolcs.nagy@arm.com>

Previously if user requested S stack and G guard when creating a
thread, the total mapping was S and the actual available stack was
S - G - static_tls, which is not what the user requested.

This patch fixes the guard size accounting by pretending the user
requested S+G stack.  This way all later logic works out except
when reporting the user requested stack size (pthread_getattr_np)
or when computing the minimal stack size (__pthread_get_minstack).

Normally this will increase thread stack allocations by one page.
TLS accounting is not affected, that will require a separate fix.

	[BZ #22637]
	* nptl/descr.h (stackblock, stackblock_size): Update comments.
	* nptl/allocatestack.c (allocate_stack): Add guardsize to stacksize.
	* nptl/nptl-init.c (__pthread_get_minstack): Remove guardsize from
	stacksize.
	* nptl/pthread_getattr_np.c (pthread_getattr_np): Likewise.

(cherry picked from commit 630f4cc3aa019ede55976ea561f1a7af2f068639)

2018-01-08  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	[BZ #22637]
	* nptl/descr.h (stackblock, stackblock_size): Update comments.
	* nptl/allocatestack.c (allocate_stack): Add guardsize to stacksize.
	* nptl/nptl-init.c (__pthread_get_minstack): Remove guardsize from
	stacksize.
	* nptl/pthread_getattr_np.c (pthread_getattr_np): Likewise.

Comments

Florian Weimer Jan. 15, 2018, 3:20 p.m. UTC | #1 
* Florian Weimer:

> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
> Previously if user requested S stack and G guard when creating a
> thread, the total mapping was S and the actual available stack was
> S - G - static_tls, which is not what the user requested.
>
> This patch fixes the guard size accounting by pretending the user
> requested S+G stack.  This way all later logic works out except
> when reporting the user requested stack size (pthread_getattr_np)
> or when computing the minimal stack size (__pthread_get_minstack).
>
> Normally this will increase thread stack allocations by one page.
> TLS accounting is not affected, that will require a separate fix.
>
> 	[BZ #22637]
> 	* nptl/descr.h (stackblock, stackblock_size): Update comments.
> 	* nptl/allocatestack.c (allocate_stack): Add guardsize to stacksize.
> 	* nptl/nptl-init.c (__pthread_get_minstack): Remove guardsize from
> 	stacksize.
> 	* nptl/pthread_getattr_np.c (pthread_getattr_np): Likewise.
>
> (cherry picked from commit 630f4cc3aa019ede55976ea561f1a7af2f068639)

Please disregard.  Should have gone to libc-stable.
diff mbox

Patch

diff --git a/NEWS b/NEWS
index 7f88e9e310..3a719e81e3 100644
--- a/NEWS
+++ b/NEWS
@@ -101,6 +101,7 @@  The following bugs are resolved with this release:
   [22325] glibc: Memory leak in glob with GLOB_TILDE (CVE-2017-15671)
   [22375] malloc returns pointer from tcache instead of NULL (CVE-2017-17426)
   [22627] $ORIGIN in $LD_LIBRARY_PATH is substituted twice
+  [22637] nptl: Fix stack guard size accounting
   [22679] getcwd(3) can succeed without returning an absolute path
     (CVE-2018-1000001)
 
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 1a760e92e5..e351ce9d99 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -533,6 +533,10 @@  allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
       /* Make sure the size of the stack is enough for the guard and
 	 eventually the thread descriptor.  */
       guardsize = (attr->guardsize + pagesize_m1) & ~pagesize_m1;
+      if (guardsize < attr->guardsize || size + guardsize < guardsize)
+	/* Arithmetic overflow.  */
+	return EINVAL;
+      size += guardsize;
       if (__builtin_expect (size < ((guardsize + __static_tls_size
 				     + MINIMAL_REST_STACK + pagesize_m1)
 				    & ~pagesize_m1),
diff --git a/nptl/descr.h b/nptl/descr.h
index c83b17b674..82dab056e2 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -380,9 +380,9 @@  struct pthread
   /* Machine-specific unwind info.  */
   struct _Unwind_Exception exc;
 
-  /* If nonzero pointer to area allocated for the stack and its
-     size.  */
+  /* If nonzero, pointer to the area allocated for the stack and guard. */
   void *stackblock;
+  /* Size of the stackblock area including the guard.  */
   size_t stackblock_size;
   /* Size of the included guard area.  */
   size_t guardsize;
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 869e926f17..e5c0bdfbeb 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -473,8 +473,5 @@  strong_alias (__pthread_initialize_minimal_internal,
 size_t
 __pthread_get_minstack (const pthread_attr_t *attr)
 {
-  struct pthread_attr *iattr = (struct pthread_attr *) attr;
-
-  return (GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN
-	  + iattr->guardsize);
+  return GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN;
 }
diff --git a/nptl/pthread_getattr_np.c b/nptl/pthread_getattr_np.c
index 06093b3d92..210a3f8a1f 100644
--- a/nptl/pthread_getattr_np.c
+++ b/nptl/pthread_getattr_np.c
@@ -57,9 +57,12 @@  pthread_getattr_np (pthread_t thread_id, pthread_attr_t *attr)
   /* The sizes are subject to alignment.  */
   if (__glibc_likely (thread->stackblock != NULL))
     {
-      iattr->stacksize = thread->stackblock_size;
+      /* The stack size reported to the user should not include the
+	 guard size.  */
+      iattr->stacksize = thread->stackblock_size - thread->guardsize;
 #if _STACK_GROWS_DOWN
-      iattr->stackaddr = (char *) thread->stackblock + iattr->stacksize;
+      iattr->stackaddr = (char *) thread->stackblock
+			 + thread->stackblock_size;
 #else
       iattr->stackaddr = (char *) thread->stackblock;
 #endif