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][BZ #12416] Use stack boundaries from /proc/PID/maps tomake stack executable


Hi,

Using __libc_stack_end as the end of stack to mark an executable stack
misses pages at the end of stack that are used to store program
arguments, environment variables and auxiliary elf information. Due to
this, the stack vma gets split and later requests for stack address
using pthread_getattr_np() return a different value than before.

This behaviour can be seen using the reproducer in bz#12416.

This patch fixes an inconsistency between the treatment of stack size
in pthread_getattr_np and _dl_make_stack_executable where the former
used __libc_stack_end to find the stack vma in /proc/PID/maps and used
the real stack end while the latter used the __libc_stack_end itself as
the stack end.

I have modified tst-execstack to test this in make check. Testsuite
reports no new regressions from this patch.

Regards,
Siddhesh

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

	[BZ #12416]
	* elf/tst-execstack.c (get_main_stack_bounds): New function.
	(do_test): Modify test to also verify that main stack bounds do
	not change after execstack-dso.so is dlopen'd.
	* sysdeps/unix/sysv/linux/dl-execstack.c (__strtoul_internal):
	Declare extern.
	(__read_one_line, __get_map_bounds): New functions.
	(_dl_make_stack_executable): Use stack bounds
	from /proc/PID/maps if available.
diff --git a/elf/tst-execstack.c b/elf/tst-execstack.c
index 6632e53..d9fa430 100644
--- a/elf/tst-execstack.c
+++ b/elf/tst-execstack.c
@@ -46,6 +46,23 @@ waiter_thread (void *arg)
 }
 #endif
 
+static void
+get_main_stack_bounds (void **from, void **to)
+{
+  FILE *proc = fopen ("/proc/self/maps", "r");
+  char *l = NULL;
+  size_t size;
+  while (getline (&l, &size, proc) != -1)
+    {
+      sscanf (l, "%p-%p", from, to);
+      if (strstr (l, "[stack]"))
+	return;
+    }
+
+  /* There should at least be one [stack].  */
+  abort ();
+}
+
 
 static bool allow_execstack = true;
 
@@ -84,6 +101,9 @@ do_test (void)
   printf ("executable stacks %sallowed\n", allow_execstack ? "" : "not ");
 
   static void *f;		/* Address of this is used in other threads. */
+  void *old_from, *old_to, *new_from, *new_to;
+
+  get_main_stack_bounds (&old_from, &old_to);
 
 #if USE_PTHREADS
   /* Create some threads while stacks are nonexecutable.  */
@@ -122,6 +142,17 @@ do_test (void)
       return 1;
     }
 
+  get_main_stack_bounds (&new_from, &new_to);
+
+  /* Make sure that the stack permissions change worked on the entire stack
+  vma.  */
+  if (old_from != new_from || old_to != new_to)
+    {
+      printf ("Stack vma has split:: Old:%p-%p, New:%p-%p",
+	      old_from, old_to, new_from, new_to);
+      return 1;
+    }
+
   /* Test if that really made our stack executable.
      The `tryme' function should crash if not.  */
 
diff --git a/sysdeps/unix/sysv/linux/dl-execstack.c b/sysdeps/unix/sysv/linux/dl-execstack.c
index 6408adc..16f02d8 100644
--- a/sysdeps/unix/sysv/linux/dl-execstack.c
+++ b/sysdeps/unix/sysv/linux/dl-execstack.c
@@ -24,20 +24,110 @@
 #include <stackinfo.h>
 #include <caller.h>
 #include <sysdep.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <alloca.h>
 
 #include <kernel-features.h>
 
 
 extern int __stack_prot attribute_relro attribute_hidden;
 
+extern unsigned long int weak_function __strtoul_internal (const char *nptr,
+							   char **endptr,
+							   int base,
+							   int group);
+
+
+static int
+__read_one_line (int fd, char **ret, char *buf, int buflen,
+                 int *buf_offset)
+{
+  while (1)
+    {
+      char *line_end = NULL;
+      char *tmp;
+      int bytes = read (fd, buf + *buf_offset, buflen - *buf_offset - 1);
+
+      if (bytes == 0 && *buf_offset == 0)
+	return 0;
+
+      buf[*buf_offset + bytes] = 0;
+
+      line_end = strchr (buf, '\n');
+
+      if (line_end)
+        {
+          int len, i;
+
+          *line_end = '\0';
+          len = strlen (buf) + 1;
+	  len += (*ret) ? strlen (*ret) : 0;
+          tmp = realloc (*ret, len);
+
+	  if (tmp == NULL)
+	    return 0;
+
+	  *ret = tmp;
+          memcpy (*ret, buf, len);
+
+          for (i = len; i < *buf_offset + bytes; i++)
+            buf [i - len] = buf [i];
+          *buf_offset = i - len;
+          return len;
+        }
+      else
+        {
+          /* Our buffer is too small.  Dump it into ret and reallocate.  */
+          int old_len = *ret ? strlen (*ret) : 0;
+          tmp = realloc (*ret, old_len + strlen (buf) + 1);
+
+          if (tmp == NULL)
+            return 0;
+
+          *ret = tmp;
+          memcpy (*ret + old_len - 1, buf, strlen (buf) + 1);
+	  *buf_offset = 0;
+        }
+    }
+}
+
+static int
+__get_map_bounds (char *line, uintptr_t *from, uintptr_t *to)
+{
+  char *end;
+  char *from_str = line;
+  char *to_str = strchr (line, '-');
+  char *buf = alloca (sizeof (unsigned long) * 2 + 3);
+
+  if (!to_str)
+    return 0;
+
+  *to_str++ = '\0';
+  end = strchr (to_str, ' ');
+
+  if (!end)
+    return 0;
+
+  *end = '\0';
+
+  memcpy (buf, "0x", 2);
+
+  memcpy (&buf[2], from_str, strlen (from_str) + 1);
+  *from = __strtoul_internal (buf, NULL, 0, 0);
+  memcpy (&buf[2], to_str, strlen (to_str) + 1);
+  *to = __strtoul_internal (buf, NULL, 0, 0);
+
+  return 1;
+}
 
 int
 internal_function
 _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));
+  uintptr_t page;
+  uintptr_t real_stack_end = 0;
   int result = 0;
 
   /* Challenge the caller.  */
@@ -46,6 +136,55 @@ _dl_make_stack_executable (void **stack_endp)
       || __builtin_expect (*stack_endp != __libc_stack_end, 0))
     return EPERM;
 
+  /* Get the real end of stack from proc/PID/maps.  */
+  int fd = open ("/proc/self/maps", O_RDONLY | O_CLOEXEC);
+  if (fd >= 0)
+    {
+      char *line = NULL;
+      int buf_offset = 0;
+      while (1)
+        {
+	  char buf[255];
+	  if (__read_one_line(fd, &line, buf, sizeof (buf), &buf_offset) == 0)
+	    break;
+          uintptr_t from;
+          uintptr_t to;
+	  if (__get_map_bounds (line, &from, &to) == 0)
+	    goto continue_loop;
+          if (from <= (uintptr_t) *stack_endp
+              && (uintptr_t) *stack_endp < to)
+            {
+#if _STACK_GROWS_DOWN
+              real_stack_end = to;
+#elif _STACK_GROWS_UP
+	      real_stack_end = from;
+#else
+# error "Define either _STACK_GROWS_DOWN or _STACK_GROWS_UP"
+#endif
+              break;
+            }
+	continue_loop:
+	  free (line);
+	  line = NULL;
+        }
+
+      free (line);
+      close (fd);
+    }
+
+  /* Fall back to __libc_stack_end.  */
+  if (__builtin_expect (real_stack_end == 0, 0))
+    /* This gives us the highest/lowest page that needs to be changed.  */
+    page = ((uintptr_t) *stack_endp & -(intptr_t) GLRO(dl_pagesize));
+  else
+    /* Pull back by a page so that we're not trying to mprotect the next
+    vma.  Also, we can use the value without worrying about page alignment
+    since vmas are always page aligned.  This is not necessary when
+    _STACK_GROWS_UP.  */
+#if _STACK_GROWS_DOWN
+    page = real_stack_end - GLRO(dl_pagesize);
+#endif
+
   /* Newer Linux kernels support a flag to make our job easy.  */
 #if defined  PROT_GROWSDOWN || defined PROT_GROWSUP
 # if __ASSUME_PROT_GROWSUPDOWN == 0
-- 
1.7.7.6


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