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]

[PATCH 2/2 Take-2][BZ #12416] Use stack boundaries from/proc/PID/maps to make stack executable


On Wed, 25 Apr 2012 13:34:24 -0700 (PDT), Roland wrote:
> Perhaps it should work in a different way.  It's nasty to
> require /proc access, and all the io plus stdio overhead et al might
> make it more costly than another method.  It could use something like
> the mprotect-probing technique to locate the hole below the stack
> that linux/dl-execstack.c uses when PROT_GROWSDOWN is not available.

I have written a new patch based on this idea. I have consolidated the
mprotect-probing code into a function to make the code a bit cleaner,
since the same logic is being used repeatedly. I have also updated the
test case to not use procfs and use pthread_getattr_np instead. I have
verified (on x86_64) the test case with and without this fix. I have
also verified that there are no regressions in the testsuite due to
this fix.

Regards,
Siddhesh

ChangeLog:

2012-04-27  Siddhesh Poyarekar  <siddhesh@redhat.com>

	[BZ #12416]
	Based on idea by Roland McGrath.
	* elf/tst-execstack.c (do_test): Check that pthread_getattr_np
	returns the same end of stack before and after stack is made
	executable.
	* sysdeps/unix/sysv/linux/dl-execstack.c
	(_dl_make_stack_executable): Also mark pages below (or above if
	_STACK_GROWS_UP) as executable. Consolidate code to
	incrementally mark stack executable to ...
	(_incremental_make_stack_executable): ... a new function.
diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
index 6632e53..ca0df1e 100644
--- a/elf/tst-execstack.c
+++ b/elf/tst-execstack.c
@@ -107,6 +107,17 @@ do_test (void)
 
   print_maps ();
 
+#if USE_PTHREADS
+  void *old_stack_addr, *new_stack_addr;
+  size_t stack_size;
+  pthread_t me = pthread_self ();
+  pthread_attr_t attr;
+
+  pthread_getattr_np (me, &attr);
+  pthread_attr_getstack (&attr, &old_stack_addr, &stack_size);
+  old_stack_addr += stack_size;
+#endif
+
   /* Loading this module should force stacks to become executable.  */
   void *h = dlopen ("tst-execstack-mod.so", RTLD_LAZY);
   if (h == NULL)
@@ -129,6 +140,21 @@ do_test (void)
 
   print_maps ();
 
+#if USE_PTHREADS
+  pthread_getattr_np (me, &attr);
+  pthread_attr_getstack (&attr, &new_stack_addr, &stack_size);
+  new_stack_addr += stack_size;
+
+  /* Make sure that the stack address did not change.  */
+  if (old_stack_addr != new_stack_addr)
+    {
+      printf ("Stack end changed, old: %p, new: %p\n",
+	      old_stack_addr, new_stack_addr);
+      return 1;
+    }
+  printf ("Stack address remains the same: %p\n", old_stack_addr);
+#endif
+
   /* Test that growing the stack region gets new executable pages too.  */
   deeper ((void (*) (void)) f);
 
diff --git a/sysdeps/unix/sysv/linux/dl-execstack.c b/sysdeps/unix/sysv/linux/dl-execstack.c
index 6408adc..cd4fa8a 100644
--- a/sysdeps/unix/sysv/linux/dl-execstack.c
+++ b/sysdeps/unix/sysv/linux/dl-execstack.c
@@ -30,6 +30,52 @@
 
 extern int __stack_prot attribute_relro attribute_hidden;
 
+/* Mark the stack as executable, a few pages at a time.  If we encounter an
+   ENOMEM, we try to mprotect lower number of pages till we're sure that we
+   have reached the end or beginning of stack.  DIRECTION is -1 for down and 1
+   for up, which allows us to go to either side of the page address.  */
+static int
+_incremental_make_stack_executable (uintptr_t page, size_t size, int *result,
+				    int direction)
+{
+  /* There is always a hole in the address space below the bottom of the
+     stack.  So when we make an mprotect call that starts below the bottom
+     of the stack, it will include the hole and fail with ENOMEM.
+
+     We start with a random guess at how deep the stack might have gotten
+     so as to have extended the GROWSDOWN mapping to lower pages.  */
+
+  if (direction == -1)
+    page -= size;
+
+  while (1)
+    {
+      if (__mprotect ((void *) page, size,
+		      __stack_prot & ~PROT_GROWSDOWN) == 0)
+	/* We got this chunk changed; loop to do another chunk below.  */
+	page += (long) direction * size;
+      else
+	{
+	  if (errno != ENOMEM)	/* Unexpected failure mode.  */
+	    {
+	      *result = errno;
+	      return 1;
+	    }
+
+	  if (size == GLRO(dl_pagesize))
+	    /* We just tried to mprotect the top hole page and failed.
+	       We are done.  */
+	    break;
+
+	  /* Our mprotect call failed because it started below the lowest
+	     stack page.  Try again on just the top half of that region.  */
+	  size /= 2;
+	  if (direction == -1)
+	    page += size;
+	}
+    }
+    return 0;
+}
 
 int
 internal_function
@@ -38,6 +84,8 @@ _dl_make_stack_executable (void **stack_endp)
   /* This gives us the highest/lowest page that needs to be changed.  */
   uintptr_t page = ((uintptr_t) *stack_endp
 		    & -(intptr_t) GLRO(dl_pagesize));
+  size_t size = GLRO(dl_pagesize) * 8;
+  int direction;
   int result = 0;
 
   /* Challenge the caller.  */
@@ -68,75 +116,28 @@ _dl_make_stack_executable (void **stack_endp)
     }
 #endif
 
-  /* There is always a hole in the address space below the bottom of the
-     stack.  So when we make an mprotect call that starts below the bottom
-     of the stack, it will include the hole and fail with ENOMEM.
-
-     We start with a random guess at how deep the stack might have gotten
-     so as to have extended the GROWSDOWN mapping to lower pages.  */
-
-#if __ASSUME_PROT_GROWSUPDOWN == 0
-  size_t size = GLRO(dl_pagesize) * 8;
-
-# if _STACK_GROWS_DOWN
-  page = page + GLRO(dl_pagesize) - size;
-  while (1)
-    {
-      if (__mprotect ((void *) page, size,
-		      __stack_prot & ~PROT_GROWSDOWN) == 0)
-	/* We got this chunk changed; loop to do another chunk below.  */
-	page -= size;
-      else
-	{
-	  if (errno != ENOMEM)	/* Unexpected failure mode.  */
-	    {
-	      result = errno;
-	      goto out;
-	    }
-
-	  if (size == GLRO(dl_pagesize))
-	    /* We just tried to mprotect the top hole page and failed.
-	       We are done.  */
-	    break;
-
-	  /* Our mprotect call failed because it started below the lowest
-	     stack page.  Try again on just the top half of that region.  */
-	  size /= 2;
-	  page += size;
-	}
-    }
-
-# elif _STACK_GROWS_UP
-  while (1)
-    {
-      if (__mprotect ((void *) page, size, __stack_prot & ~PROT_GROWSUP) == 0)
-	/* We got this chunk changed; loop to do another chunk below.  */
-	page += size;
-      else
-	{
-	  if (errno != ENOMEM)	/* Unexpected failure mode.  */
-	    {
-	      result = errno;
-	      goto out;
-	    }
-
-	  if (size == GLRO(dl_pagesize))
-	    /* We just tried to mprotect the lowest hole page and failed.
-	       We are done.  */
-	    break;
-
-	  /* Our mprotect call failed because it extended past the highest
-	     stack page.  Try again on just the bottom half of that region.  */
-	  size /= 2;
-	}
-    }
+#if _STACK_GROWS_DOWN
+  page += GLRO(dl_pagesize);
+  direction = -1;
+#elif _STACK_GROWS_UP
+  direction = 1;
+#else
+# error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP"
+#endif
 
-# else
-#  error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP"
+# if __ASSUME_PROT_GROWSUPDOWN == 0
+  if (_incremental_make_stack_executable (page, size, &result, direction))
+    goto out;
 # endif
-#endif
 
  return_success:
+
+  /* We need to mark the vma below the stack end as well, since the kernel
+     sets up some pages with elf info, environment variables and program
+     arguments below the end.  Not doing this will split the vma.  */
+  if (_incremental_make_stack_executable (page, size, &result, -direction))
+    goto out;
+
   /* Clear the address.  */
   *stack_endp = NULL;
 
-- 
1.7.7.6


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