From 569333293aeefbe792826cd59140dc23704018c4 Mon Sep 17 00:00:00 2001 From: "Mor-sarid, Nitzan" Date: Fri, 11 Sep 2015 05:31:36 +0000 Subject: [PATCH] Fix the way to get main thread stack start address. For previous way to get the stack using the [stack] string from /proc/self/task//maps is not enough. On x86/x86_64, if an alternative signal stack is used while a task switch happens, the [stack] indicator may no longer be correct. Instead, stack_start from /proc/self/stat which is always inside the main stack, is used to find the main stack in /proc/self/maps. Change-Id: Ieb010e71518b57560d541cd3b3563e5aa9660750 Signed-off-by: Nitzan Mor-sarid Signed-off-by: Mingwei Shi --- libc/bionic/pthread_attr.cpp | 53 ++++++++++++++++++++++++------- tests/pthread_test.cpp | 61 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 12 deletions(-) diff --git a/libc/bionic/pthread_attr.cpp b/libc/bionic/pthread_attr.cpp index 7ad34311e..cfa58fc88 100644 --- a/libc/bionic/pthread_attr.cpp +++ b/libc/bionic/pthread_attr.cpp @@ -114,6 +114,36 @@ int pthread_attr_setstack(pthread_attr_t* attr, void* stack_base, size_t stack_s return 0; } +static uintptr_t __get_main_stack_startstack() { + FILE* fp = fopen("/proc/self/stat", "re"); + if (fp == nullptr) { + __libc_fatal("couldn't open /proc/self/stat: %s", strerror(errno)); + } + + char line[BUFSIZ]; + if (fgets(line, sizeof(line), fp) == nullptr) { + __libc_fatal("couldn't read /proc/self/stat: %s", strerror(errno)); + } + + fclose(fp); + + // See man 5 proc. There's no reason comm can't contain ' ' or ')', + // so we search backwards for the end of it. We're looking for this field: + // + // startstack %lu (28) The address of the start (i.e., bottom) of the stack. + uintptr_t startstack = 0; + const char* end_of_comm = strrchr(line, ')'); + if (sscanf(end_of_comm + 1, " %*c " + "%*d %*d %*d %*d %*d " + "%*u %*u %*u %*u %*u %*u %*u " + "%*d %*d %*d %*d %*d %*d " + "%*u %*u %*d %*u %*u %*u %" SCNuPTR, &startstack) != 1) { + __libc_fatal("couldn't parse /proc/self/stat"); + } + + return startstack; +} + static int __pthread_attr_getstack_main_thread(void** stack_base, size_t* stack_size) { ErrnoRestorer errno_restorer; @@ -127,20 +157,19 @@ static int __pthread_attr_getstack_main_thread(void** stack_base, size_t* stack_ stack_limit.rlim_cur = 8 * 1024 * 1024; } - // It shouldn't matter which thread we are because we're just looking for "[stack]", but - // valgrind seems to mess with the stack enough that the kernel will report "[stack:pid]" - // instead if you look in /proc/self/maps, so we need to look in /proc/pid/task/pid/maps. - char path[64]; - snprintf(path, sizeof(path), "/proc/self/task/%d/maps", getpid()); - FILE* fp = fopen(path, "re"); - if (fp == NULL) { - return errno; + // Ask the kernel where our main thread's stack started. + uintptr_t startstack = __get_main_stack_startstack(); + + // Hunt for the region that contains that address. + FILE* fp = fopen("/proc/self/maps", "re"); + if (fp == nullptr) { + __libc_fatal("couldn't open /proc/self/maps"); } char line[BUFSIZ]; while (fgets(line, sizeof(line), fp) != NULL) { - if (ends_with(line, " [stack]\n")) { - uintptr_t lo, hi; - if (sscanf(line, "%" SCNxPTR "-%" SCNxPTR, &lo, &hi) == 2) { + uintptr_t lo, hi; + if (sscanf(line, "%" SCNxPTR "-%" SCNxPTR, &lo, &hi) == 2) { + if (lo <= startstack && startstack <= hi) { *stack_size = stack_limit.rlim_cur; *stack_base = reinterpret_cast(hi - *stack_size); fclose(fp); @@ -148,7 +177,7 @@ static int __pthread_attr_getstack_main_thread(void** stack_base, size_t* stack_ } } } - __libc_fatal("No [stack] line found in \"%s\"!", path); + __libc_fatal("Stack not found in /proc/self/maps"); } int pthread_attr_getstack(const pthread_attr_t* attr, void** stack_base, size_t* stack_size) { diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index eeb154143..3c686ef20 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -1220,6 +1220,67 @@ TEST(pthread, pthread_attr_getstack__main_thread) { #endif } +struct GetStackSignalHandlerArg { + volatile bool done; + void* signal_handler_sp; + void* main_stack_base; + size_t main_stack_size; +}; + +static GetStackSignalHandlerArg getstack_signal_handler_arg; + +static void getstack_signal_handler(int sig) { + ASSERT_EQ(SIGUSR1, sig); + // Use sleep() to make current thread be switched out by the kernel to provoke the error. + sleep(1); + pthread_attr_t attr; + ASSERT_EQ(0, pthread_getattr_np(pthread_self(), &attr)); + void* stack_base; + size_t stack_size; + ASSERT_EQ(0, pthread_attr_getstack(&attr, &stack_base, &stack_size)); + getstack_signal_handler_arg.signal_handler_sp = &attr; + getstack_signal_handler_arg.main_stack_base = stack_base; + getstack_signal_handler_arg.main_stack_size = stack_size; + getstack_signal_handler_arg.done = true; +} + +// The previous code obtained the main thread's stack by reading the entry in +// /proc/self/task//maps that was labeled [stack]. Unfortunately, on x86/x86_64, the kernel +// relies on sp0 in task state segment(tss) to label the stack map with [stack]. If the kernel +// switches a process while the main thread is in an alternate stack, then the kernel will label +// the wrong map with [stack]. This test verifies that when the above situation happens, the main +// thread's stack is found correctly. +TEST(pthread, pthread_attr_getstack_in_signal_handler) { + const size_t sig_stack_size = 16 * 1024; + void* sig_stack = mmap(NULL, sig_stack_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, + -1, 0); + ASSERT_NE(MAP_FAILED, sig_stack); + stack_t ss; + ss.ss_sp = sig_stack; + ss.ss_size = sig_stack_size; + ss.ss_flags = 0; + stack_t oss; + ASSERT_EQ(0, sigaltstack(&ss, &oss)); + + ScopedSignalHandler handler(SIGUSR1, getstack_signal_handler, SA_ONSTACK); + getstack_signal_handler_arg.done = false; + kill(getpid(), SIGUSR1); + ASSERT_EQ(true, getstack_signal_handler_arg.done); + + // Verify if the stack used by the signal handler is the alternate stack just registered. + ASSERT_LE(sig_stack, getstack_signal_handler_arg.signal_handler_sp); + ASSERT_GE(reinterpret_cast(sig_stack) + sig_stack_size, + getstack_signal_handler_arg.signal_handler_sp); + + // Verify if the main thread's stack got in the signal handler is correct. + ASSERT_LE(getstack_signal_handler_arg.main_stack_base, &ss); + ASSERT_GE(reinterpret_cast(getstack_signal_handler_arg.main_stack_base) + + getstack_signal_handler_arg.main_stack_size, reinterpret_cast(&ss)); + + ASSERT_EQ(0, sigaltstack(&oss, nullptr)); + ASSERT_EQ(0, munmap(sig_stack, sig_stack_size)); +} + static void pthread_attr_getstack_18908062_helper(void*) { char local_variable; pthread_attr_t attributes;