diff mbox

[V8,1/6] oslib-posix: add helpers for stack alloc and free

Message ID 1474890272-22952-2-git-send-email-pl@kamp.de (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Lieven Sept. 26, 2016, 11:44 a.m. UTC
the allocated stack will be adjusted to the minimum supported stack size
by the OS and rounded up to be a multiple of the system pagesize.
Additionally an architecture dependent guard page is added to the stack
to catch stack overflows.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/sysemu/os-posix.h | 27 +++++++++++++++++++++++++++
 util/oslib-posix.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

Comments

Kevin Wolf Sept. 26, 2016, 1:44 p.m. UTC | #1
Am 26.09.2016 um 13:44 hat Peter Lieven geschrieben:
> the allocated stack will be adjusted to the minimum supported stack size
> by the OS and rounded up to be a multiple of the system pagesize.
> Additionally an architecture dependent guard page is added to the stack
> to catch stack overflows.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/sysemu/os-posix.h | 27 +++++++++++++++++++++++++++
>  util/oslib-posix.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
> index 9c7dfdf..4a0f493 100644
> --- a/include/sysemu/os-posix.h
> +++ b/include/sysemu/os-posix.h
> @@ -60,4 +60,31 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
>  
>  bool is_daemonized(void);
>  
> +/**
> + * qemu_alloc_stack:
> + * @sz: pointer to a size_t holding the requested stack size
> + *
> + * 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()). This function also inserts an
> + * additional guard page to catch a potential stack overflow.
> + * Note that the useable stack memory can be greater than the
> + * requested stack size due to alignment and minimal stack size
> + * restrictions. In this case the value of sz is adjusted.
> + *
> + * The allocated stack must be freed with qemu_free_stack().
> + *
> + * Returns: pointer to (the lowest address of) the stack memory.

Not quite. It's the pointer to the lowest address of the guard page,
while the returned stack size doesn't include the guard page. This is an
awkward interface, and consequently patch 3 fails to use it correctly.

So you end up with something like:

    |GGGG|....|....|....|
     **** **** ****

    G = guard page
    . = allocated stack page
    * = stack as used for makecontext()

That is, the guard page is included in the stack used to create the
coroutine context, and the last page stays unused. On systems where we
only allocate a single page for the stack, this obviously means that the
tests still fail.

Kevin
Peter Lieven Sept. 26, 2016, 2:43 p.m. UTC | #2
Am 26.09.2016 um 15:44 schrieb Kevin Wolf:
> Am 26.09.2016 um 13:44 hat Peter Lieven geschrieben:
>> the allocated stack will be adjusted to the minimum supported stack size
>> by the OS and rounded up to be a multiple of the system pagesize.
>> Additionally an architecture dependent guard page is added to the stack
>> to catch stack overflows.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   include/sysemu/os-posix.h | 27 +++++++++++++++++++++++++++
>>   util/oslib-posix.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 70 insertions(+)
>>
>> diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
>> index 9c7dfdf..4a0f493 100644
>> --- a/include/sysemu/os-posix.h
>> +++ b/include/sysemu/os-posix.h
>> @@ -60,4 +60,31 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
>>   
>>   bool is_daemonized(void);
>>   
>> +/**
>> + * qemu_alloc_stack:
>> + * @sz: pointer to a size_t holding the requested stack size
>> + *
>> + * 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()). This function also inserts an
>> + * additional guard page to catch a potential stack overflow.
>> + * Note that the useable stack memory can be greater than the
>> + * requested stack size due to alignment and minimal stack size
>> + * restrictions. In this case the value of sz is adjusted.
>> + *
>> + * The allocated stack must be freed with qemu_free_stack().
>> + *
>> + * Returns: pointer to (the lowest address of) the stack memory.
> Not quite. It's the pointer to the lowest address of the guard page,
> while the returned stack size doesn't include the guard page. This is an
> awkward interface, and consequently patch 3 fails to use it correctly.
>
> So you end up with something like:
>
>      |GGGG|....|....|....|
>       **** **** ****
>
>      G = guard page
>      . = allocated stack page
>      * = stack as used for makecontext()
>
> That is, the guard page is included in the stack used to create the
> coroutine context, and the last page stays unused. On systems where we
> only allocate a single page for the stack, this obviously means that the
> tests still fail.

you are right. so I should adjust the size to allocsz instead?

the other option would be to keep version 7 of this series and
adjust the COROUTINE_SIZE to MAX(2*pagesize(), 1 << 16) to
avoid the problem?

Peter
Kevin Wolf Sept. 26, 2016, 2:51 p.m. UTC | #3
Am 26.09.2016 um 16:43 hat Peter Lieven geschrieben:
> Am 26.09.2016 um 15:44 schrieb Kevin Wolf:
> >Am 26.09.2016 um 13:44 hat Peter Lieven geschrieben:
> >>the allocated stack will be adjusted to the minimum supported stack size
> >>by the OS and rounded up to be a multiple of the system pagesize.
> >>Additionally an architecture dependent guard page is added to the stack
> >>to catch stack overflows.
> >>
> >>Signed-off-by: Peter Lieven <pl@kamp.de>
> >>---
> >>  include/sysemu/os-posix.h | 27 +++++++++++++++++++++++++++
> >>  util/oslib-posix.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 70 insertions(+)
> >>
> >>diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
> >>index 9c7dfdf..4a0f493 100644
> >>--- a/include/sysemu/os-posix.h
> >>+++ b/include/sysemu/os-posix.h
> >>@@ -60,4 +60,31 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
> >>  bool is_daemonized(void);
> >>+/**
> >>+ * qemu_alloc_stack:
> >>+ * @sz: pointer to a size_t holding the requested stack size
> >>+ *
> >>+ * 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()). This function also inserts an
> >>+ * additional guard page to catch a potential stack overflow.
> >>+ * Note that the useable stack memory can be greater than the
> >>+ * requested stack size due to alignment and minimal stack size
> >>+ * restrictions. In this case the value of sz is adjusted.
> >>+ *
> >>+ * The allocated stack must be freed with qemu_free_stack().
> >>+ *
> >>+ * Returns: pointer to (the lowest address of) the stack memory.
> >Not quite. It's the pointer to the lowest address of the guard page,
> >while the returned stack size doesn't include the guard page. This is an
> >awkward interface, and consequently patch 3 fails to use it correctly.
> >
> >So you end up with something like:
> >
> >     |GGGG|....|....|....|
> >      **** **** ****
> >
> >     G = guard page
> >     . = allocated stack page
> >     * = stack as used for makecontext()
> >
> >That is, the guard page is included in the stack used to create the
> >coroutine context, and the last page stays unused. On systems where we
> >only allocate a single page for the stack, this obviously means that the
> >tests still fail.
> 
> you are right. so I should adjust the size to allocsz instead?

That's probably the easiest fix.

Kevin

> the other option would be to keep version 7 of this series and
> adjust the COROUTINE_SIZE to MAX(2*pagesize(), 1 << 16) to
> avoid the problem?
> 
> Peter
diff mbox

Patch

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..4a0f493 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,31 @@  int qemu_utimens(const char *path, const qemu_timespec *times);
 
 bool is_daemonized(void);
 
+/**
+ * qemu_alloc_stack:
+ * @sz: pointer to a size_t holding the requested stack size
+ *
+ * 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()). This function also inserts an
+ * additional guard page to catch a potential stack overflow.
+ * Note that the useable stack memory can be greater than the
+ * requested stack size due to alignment and minimal stack size
+ * restrictions. In this case the value of sz is adjusted.
+ *
+ * The allocated stack must be freed with qemu_free_stack().
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t *sz);
+
+/**
+ * qemu_free_stack:
+ * @stack: stack to free
+ * @sz: size of stack in bytes
+ *
+ * Free a stack allocated via qemu_alloc_stack().
+ */
+void qemu_free_stack(void *stack, size_t sz);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f2d4e9e..7d053b8 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -499,3 +499,46 @@  pid_t qemu_fork(Error **errp)
     }
     return pid;
 }
+
+void *qemu_alloc_stack(size_t *sz)
+{
+    void *ptr, *guardpage;
+    size_t pagesz = getpagesize();
+    size_t allocsz;
+#ifdef _SC_THREAD_STACK_MIN
+    /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+    long min_stack_sz = sysconf(_SC_THREAD_STACK_MIN);
+    *sz = MAX(MAX(min_stack_sz, 0), *sz);
+#endif
+    /* adjust stack size to a multiple of the page size */
+    *sz = ROUND_UP(*sz, pagesz);
+    /* allocate one extra page for the guard page */
+    allocsz = *sz + getpagesize();
+
+    ptr = mmap(NULL, allocsz, PROT_READ | PROT_WRITE,
+               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    if (ptr == MAP_FAILED) {
+        abort();
+    }
+
+#if defined(HOST_IA64)
+    /* separate register stack */
+    guardpage = ptr + (((allocsz - pagesz) / 2) & ~pagesz);
+#elif defined(HOST_HPPA)
+    /* stack grows up */
+    guardpage = ptr + allocsz - pagesz;
+#else
+    /* stack grows down */
+    guardpage = ptr;
+#endif
+    if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+        abort();
+    }
+
+    return ptr;
+}
+
+void qemu_free_stack(void *stack, size_t sz)
+{
+    munmap(stack, sz + getpagesize());
+}