[Qemu-devel] [PATCH] oslib-posix: New qemu_alloc_stack() to allocate stack with correct perms

* [Qemu-devel] [PATCH] oslib-posix: New qemu_alloc_stack() to allocate stack with correct perms
@ 2016-06-17 14:11 Peter Maydell
  2016-06-17 16:12 ` Richard Henderson
  2016-06-18 13:55 ` Laszlo Ersek
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Maydell @ 2016-06-17 14:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: patches, Aurelien Jarno, Leon Alrae, Michael Tokarev,
	Daniel P. Berrange, Paolo Bonzini, Richard Henderson

Some architectures require the stack to be executable; notably
this includes MIPS, because the kernel's floating point emulator
may try to put trampoline code on the stack to handle some cases.
(See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815409
for an example of this causing QEMU to crash.)

Create a utility function qemu_alloc_stack() which allocates a
block of memory for use as a stack with the correct permissions.
Since we would prefer to make the stack non-executable if we can
as a defence against code execution exploits, we detect whether
the existing stack is mapped executable. Unfortunately this
requires us to grovel through /proc/self/maps to determine the
permissions on it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This method of figuring out the correct perms for the stack is
not exactly pretty; better suggestions welcome.

NB that this utility function also gives us a handy place to put
code for allocating a guard page at the bottom of the stack, or
mapping it as MAP_GROWSDOWN, or whatever.
---
 include/sysemu/os-posix.h    | 12 ++++++++
 util/coroutine-sigaltstack.c |  2 +-
 util/coroutine-ucontext.c    |  2 +-
 util/oslib-posix.c           | 70 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..1dc9d3c 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,16 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
 
 bool is_daemonized(void);
 
+/**
+ * qemu_alloc_stack:
+ * @sz: size of required stack in bytes
+ *
+ * Allocate memory that can be used as a stack, for instance for
+ * coroutines. If the memory cannot be allocated, this function
+ * will abort (like g_malloc()).
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t sz);
+
 #endif
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index a7c3366..209f6e5 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -164,7 +164,7 @@ Coroutine *qemu_coroutine_new(void)
      */
 
     co = g_malloc0(sizeof(*co));
-    co->stack = g_malloc(stack_size);
+    co->stack = qemu_alloc_stack(stack_size);
     co->base.entry_arg = &old_env; /* stash away our jmp_buf */
 
     coTS = coroutine_get_thread_state();
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 2bb7e10..a455519 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -101,7 +101,7 @@ Coroutine *qemu_coroutine_new(void)
     }
 
     co = g_malloc0(sizeof(*co));
-    co->stack = g_malloc(stack_size);
+    co->stack = qemu_alloc_stack(stack_size);
     co->base.entry_arg = &old_env; /* stash away our jmp_buf */
 
     uc.uc_link = &old_uc;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..311093e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,3 +497,73 @@ pid_t qemu_fork(Error **errp)
     }
     return pid;
 }
+
+#if defined(__linux__)
+static int stack_prot(void)
+{
+    static int prot;
+    gchar *maps, *start, *end;
+
+    if (prot) {
+        return prot;
+    }
+
+    /* Some architectures (notably MIPS) require an executable stack, but
+     * we would prefer to avoid making the stack executable unnecessarily,
+     * to defend against code execution exploits.
+     * Check whether the current stack is executable, and follow its lead.
+     * Unfortunately to do this we have to wade through /proc/self/maps
+     * looking for the stack memory. We default to assuming we need an
+     * executable stack and remove the permission only if we can successfully
+     * confirm that non-executable is OK.
+     */
+
+    prot = PROT_READ | PROT_WRITE | PROT_EXEC;
+
+    if (!g_file_get_contents("/proc/self/maps", &maps, NULL, NULL)) {
+        return prot;
+    }
+
+    /* We are looking for a line like this:
+     *  7fffbe217000-7fffbe238000 rw-p 00000000 00:00 0          [stack]
+     * and checking whether it says 'rw-' or 'rwx'.
+     * We look forwards for [stack], then back to the preceding newline,
+     * then forwards for the rw- between the two.
+     */
+    end = g_strstr_len(maps, -1, "[stack]");
+    if (!end) {
+        return prot;
+    }
+    start = g_strrstr_len(maps, end - maps, "\n");
+    if (!start) {
+        start = maps;
+    }
+    if (g_strstr_len(start, end - start, "rw-")) {
+        prot &= ~PROT_EXEC;
+    }
+
+    return prot;
+}
+
+#else
+static int stack_prot(void)
+{
+    /* Assume an executable stack is needed, since we can't detect it. */
+    return PROT_READ | PROT_WRITE | PROT_EXEC;
+}
+#endif
+
+void *qemu_alloc_stack(size_t sz)
+{
+    /* Allocate memory for the coroutine's stack.
+     * Note that some architectures require this to be executable.
+     */
+    int prot = stack_prot();
+    void *stack = mmap(NULL, sz, prot, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+
+    if (stack == MAP_FAILED) {
+        g_error("%s: failed to allocate %zu bytes", __func__, sz);
+    }
+
+    return stack;
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread