From 595752f623ae88f7e4193a6e531a0805f1c6c4dc Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sun, 29 Mar 2015 00:03:55 -0400 Subject: [PATCH] add guard pages to the internal signal stacks Signal handlers tend to be lean, but can still overflow the (tiny) stack. Change-Id: Ia21c6453d92a9f8d1536ad01ff26a1a84c05f8fb --- libc/bionic/pthread_create.cpp | 18 ++++++++++++------ libc/bionic/pthread_exit.cpp | 2 +- libc/bionic/pthread_internal.h | 3 +++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index 66632c494..179ae95da 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -69,17 +69,23 @@ void __init_tls(pthread_internal_t* thread) { void __init_alternate_signal_stack(pthread_internal_t* thread) { // Create and set an alternate signal stack. - stack_t ss; - ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); - if (ss.ss_sp != MAP_FAILED) { - ss.ss_size = SIGSTKSZ; + void* stack_base = mmap(NULL, SIGNAL_STACK_SIZE, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); + if (stack_base != MAP_FAILED) { + // Create a guard page to catch stack overflows in signal handlers. + if (mprotect(stack_base, PAGE_SIZE, PROT_NONE) == -1) { + munmap(stack_base, SIGNAL_STACK_SIZE); + return; + } + stack_t ss; + ss.ss_sp = stack_base + PAGE_SIZE; + ss.ss_size = SIGNAL_STACK_SIZE - PAGE_SIZE; ss.ss_flags = 0; sigaltstack(&ss, NULL); - thread->alternate_signal_stack = ss.ss_sp; + thread->alternate_signal_stack = stack_base; // We can only use const static allocated string for mapped region name, as Android kernel // uses the string pointer directly when dumping /proc/pid/maps. - prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ss.ss_sp, ss.ss_size, "thread signal stack"); + prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, stack_base, SIGNAL_STACK_SIZE, "thread signal stack"); } } diff --git a/libc/bionic/pthread_exit.cpp b/libc/bionic/pthread_exit.cpp index 1de85f510..ceda93166 100644 --- a/libc/bionic/pthread_exit.cpp +++ b/libc/bionic/pthread_exit.cpp @@ -87,7 +87,7 @@ void pthread_exit(void* return_value) { sigaltstack(&ss, NULL); // Free it. - munmap(thread->alternate_signal_stack, SIGSTKSZ); + munmap(thread->alternate_signal_stack, SIGNAL_STACK_SIZE); thread->alternate_signal_stack = NULL; } diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 2151e03ed..3b91e6a19 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -130,6 +130,9 @@ __LIBC_HIDDEN__ void pthread_key_clean_all(void); */ #define PTHREAD_STACK_SIZE_DEFAULT ((1 * 1024 * 1024) - SIGSTKSZ) +/* Leave room for a guard page in the internally created signal stacks. */ +#define SIGNAL_STACK_SIZE (SIGSTKSZ + PAGE_SIZE) + /* Needed by fork. */ __LIBC_HIDDEN__ extern void __bionic_atfork_run_prepare(); __LIBC_HIDDEN__ extern void __bionic_atfork_run_child(); -- 2.11.0