On 06/17/16 16:11, Peter Maydell wrote: > 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.mayd...@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; > +} >
Shouldn't you adapt the qemu_coroutine_delete() functions in "util/coroutine-sigaltstack.c" and "util/coroutine-ucontext.c"? Both of those call g_free(co->stack). I think a qemu_free_stack() function, calling munmap(), is necessary. Thanks, Laszlo