From 53dc9dd70155fd75af744cbebecc563658c69818 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 19 Sep 2017 14:02:50 -0700 Subject: [PATCH] Improve pthread_create failure handling. Return EAGAIN rather than aborting if we fail to set up the TLS for a new thread. Add a test that uses all the VMAs so we can properly test these edge cases. Add an explicit test for pthread_attr_setdetachstate, which we use in the previous test, but other than that has no tests. Remove support for ro.logd.timestamp/persist.logd.timestamp, which doesn't seem to be used, and which prevents us from logging failures in cases where mmap fails (because we need to mmap in the system property implementation). Bug: http://b/65608572 Test: ran tests Change-Id: I9009f06546e1c2cc55eff996d08b55eff3482343 --- libc/async_safe/async_safe_log.cpp | 15 +------- libc/bionic/__libc_init_main_thread.cpp | 2 +- libc/bionic/pthread_create.cpp | 20 ++++++++--- libc/bionic/pthread_internal.h | 2 +- tests/pthread_test.cpp | 46 +++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 20 deletions(-) diff --git a/libc/async_safe/async_safe_log.cpp b/libc/async_safe/async_safe_log.cpp index d81ef34ce..78f62bdba 100644 --- a/libc/async_safe/async_safe_log.cpp +++ b/libc/async_safe/async_safe_log.cpp @@ -468,19 +468,6 @@ static int open_log_socket() { return log_fd; } -static clockid_t log_clockid() { - static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; - ScopedPthreadMutexLocker locker(&mutex); - - static CachedProperty ro_logd_timestamp("ro.logd.timestamp"); - static CachedProperty persist_logd_timestamp("persist.logd.timestamp"); - - char ch = persist_logd_timestamp.Get()[0]; - if (ch == '\0') ch = ro_logd_timestamp.Get()[0]; - - return (tolower(ch) == 'm') ? CLOCK_MONOTONIC : CLOCK_REALTIME; -} - struct log_time { // Wire format uint32_t tv_sec; uint32_t tv_nsec; @@ -501,7 +488,7 @@ int async_safe_write_log(int priority, const char* tag, const char* msg) { vec[1].iov_base = &tid; vec[1].iov_len = sizeof(tid); timespec ts; - clock_gettime(log_clockid(), &ts); + clock_gettime(CLOCK_REALTIME, &ts); log_time realtime_ts; realtime_ts.tv_sec = ts.tv_sec; realtime_ts.tv_nsec = ts.tv_nsec; diff --git a/libc/bionic/__libc_init_main_thread.cpp b/libc/bionic/__libc_init_main_thread.cpp index f3dbfa537..9cbff11af 100644 --- a/libc/bionic/__libc_init_main_thread.cpp +++ b/libc/bionic/__libc_init_main_thread.cpp @@ -70,7 +70,7 @@ void __libc_init_main_thread(KernelArgumentBlock& args) { // set up before we call any function that might get a stack check inserted. // TLS also needs to be set up before errno (and therefore syscalls) can be used. __set_tls(main_thread.tls); - __init_tls(&main_thread); + if (!__init_tls(&main_thread)) async_safe_fatal("failed to initialize TLS: %s", strerror(errno)); // Tell the kernel to clear our tid field when we exit, so we're like any other pthread. // As a side-effect, this tells us our pid (which is the same as the main thread's tid). diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index 09ae16c55..65ab92cf2 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -50,7 +50,7 @@ void __init_user_desc(struct user_desc*, bool, void*); #endif // This code is used both by each new pthread and the code that initializes the main thread. -void __init_tls(pthread_internal_t* thread) { +bool __init_tls(pthread_internal_t* thread) { // Slot 0 must point to itself. The x86 Linux kernel reads the TLS from %fs:0. thread->tls[TLS_SLOT_SELF] = thread->tls; thread->tls[TLS_SLOT_THREAD_ID] = thread; @@ -59,16 +59,25 @@ void __init_tls(pthread_internal_t* thread) { size_t allocation_size = BIONIC_TLS_SIZE + (2 * PTHREAD_GUARD_SIZE); void* allocation = mmap(nullptr, allocation_size, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (allocation == MAP_FAILED) { - async_safe_fatal("failed to allocate TLS: %s", strerror(errno)); + async_safe_format_log(ANDROID_LOG_WARN, "libc", + "pthread_create failed: couldn't allocate TLS: %s", strerror(errno)); + return false; } + prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, allocation, allocation_size, "bionic TLS guard"); + // Carve out the writable TLS section. thread->bionic_tls = reinterpret_cast(static_cast(allocation) + PTHREAD_GUARD_SIZE); if (mprotect(thread->bionic_tls, BIONIC_TLS_SIZE, PROT_READ | PROT_WRITE) != 0) { - async_safe_fatal("failed to mprotect TLS: %s", strerror(errno)); + async_safe_format_log(ANDROID_LOG_WARN, "libc", + "pthread_create failed: couldn't mprotect TLS: %s", strerror(errno)); + munmap(allocation, allocation_size); + return false; } + prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, thread->bionic_tls, BIONIC_TLS_SIZE, "bionic TLS"); + return true; } void __init_thread_stack_guard(pthread_internal_t* thread) { @@ -194,7 +203,10 @@ static int __allocate_thread(pthread_attr_t* attr, pthread_internal_t** threadp, thread->mmap_size = mmap_size; thread->attr = *attr; - __init_tls(thread); + if (!__init_tls(thread)) { + if (thread->mmap_size != 0) munmap(thread->attr.stack_base, thread->mmap_size); + return EAGAIN; + } __init_thread_stack_guard(thread); *threadp = thread; diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 77bdd8573..ad8be665c 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -115,7 +115,7 @@ class pthread_internal_t { }; __LIBC_HIDDEN__ int __init_thread(pthread_internal_t* thread); -__LIBC_HIDDEN__ void __init_tls(pthread_internal_t* thread); +__LIBC_HIDDEN__ bool __init_tls(pthread_internal_t* thread); __LIBC_HIDDEN__ void __init_thread_stack_guard(pthread_internal_t* thread); __LIBC_HIDDEN__ void __init_alternate_signal_stack(pthread_internal_t*); diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index 712a6d703..46140bbc6 100755 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -2100,3 +2100,49 @@ TEST(pthread, pthread_spinlock_smoke) { ASSERT_EQ(0, pthread_spin_unlock(&lock)); ASSERT_EQ(0, pthread_spin_destroy(&lock)); } + +TEST(pthread, pthread_attr_setdetachstate) { + pthread_attr_t attr; + ASSERT_EQ(0, pthread_attr_init(&attr)); + + ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED)); + ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE)); + ASSERT_EQ(EINVAL, pthread_attr_setdetachstate(&attr, 123)); +} + +TEST(pthread, pthread_create__mmap_failures) { + pthread_attr_t attr; + ASSERT_EQ(0, pthread_attr_init(&attr)); + ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED)); + + const auto kPageSize = sysconf(_SC_PAGE_SIZE); + + // Use up all the VMAs. By default this is 64Ki. + std::vector pages; + int prot = PROT_NONE; + while (true) { + void* page = mmap(nullptr, kPageSize, prot, MAP_ANON|MAP_PRIVATE, -1, 0); + if (page == MAP_FAILED) break; + pages.push_back(page); + prot = (prot == PROT_NONE) ? PROT_READ : PROT_NONE; + } + + // Try creating threads, freeing up a page each time we fail. + size_t EAGAIN_count = 0; + size_t i = 0; + for (; i < pages.size(); ++i) { + pthread_t t; + int status = pthread_create(&t, &attr, IdFn, nullptr); + if (status != EAGAIN) break; + ++EAGAIN_count; + ASSERT_EQ(0, munmap(pages[i], kPageSize)); + } + + // Creating a thread uses at least six VMAs: the stack, the TLS, and a guard each side of both. + // So we should have seen at least six failures. + ASSERT_GE(EAGAIN_count, 6U); + + for (; i < pages.size(); ++i) { + ASSERT_EQ(0, munmap(pages[i], kPageSize)); + } +}