[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] oslib-posix: New qemu_alloc_stack() to allocate
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH] oslib-posix: New qemu_alloc_stack() to allocate stack with correct perms |
Date: |
Sat, 18 Jun 2016 15:55:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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 <address@hidden>
> ---
> 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