diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp index 71acc45a0..881b09161 100644 --- a/libc/bionic/libc_init_common.cpp +++ b/libc/bionic/libc_init_common.cpp @@ -48,7 +48,7 @@ extern "C" int __system_properties_init(void); // Not public, but well-known in the BSDs. const char* __progname; -// Declared in +// Declared in . char** environ; // Declared in . @@ -66,7 +66,9 @@ unsigned int __page_shift = PAGE_SHIFT; * This function also stores a pointer to the kernel argument block in a TLS slot to be * picked up by the libc constructor. */ -extern "C" void __libc_init_tls(void* kernel_argument_block) { +void __libc_init_tls(KernelArgumentBlock& args) { + __libc_auxv = args.auxv; + unsigned stack_top = (__get_sp() & ~(PAGE_SIZE - 1)) + PAGE_SIZE; unsigned stack_size = 128 * 1024; unsigned stack_bottom = stack_top - stack_size; @@ -80,7 +82,7 @@ extern "C" void __libc_init_tls(void* kernel_argument_block) { static void* tls_area[BIONIC_TLS_SLOTS]; __init_tls(tls_area, &thread); - tls_area[TLS_SLOT_BIONIC_PREINIT] = kernel_argument_block; + tls_area[TLS_SLOT_BIONIC_PREINIT] = &args; } void __libc_init_common(KernelArgumentBlock& args) { diff --git a/libc/bionic/libc_init_static.cpp b/libc/bionic/libc_init_static.cpp index e5506d1f1..a6b20eb9d 100644 --- a/libc/bionic/libc_init_static.cpp +++ b/libc/bionic/libc_init_static.cpp @@ -87,9 +87,8 @@ __noreturn void __libc_init(void* raw_args, void (*onexit)(void), int (*slingshot)(int, char**, char**), structors_array_t const * const structors) { - __libc_init_tls(NULL); - KernelArgumentBlock args(raw_args); + __libc_init_tls(args); __libc_init_common(args); apply_gnu_relro(); diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c index f2a7ebe44..88a972d3c 100644 --- a/libc/bionic/pthread.c +++ b/libc/bionic/pthread.c @@ -170,13 +170,12 @@ void __init_tls(void** tls, void* thread) { } // Slot 0 must point to itself. The x86 Linux kernel reads the TLS from %fs:0. - tls[TLS_SLOT_SELF] = (void*) tls; + tls[TLS_SLOT_SELF] = tls; tls[TLS_SLOT_THREAD_ID] = thread; + // GCC looks in the TLS for the stack guard on x86, so copy it there from our global. + tls[TLS_SLOT_STACK_GUARD] = (void*) __stack_chk_guard; - // Stack guard generation may make system calls, and those system calls may fail. - // If they do, they'll try to set errno, so we can only do this after calling __set_tls. __set_tls((void*) tls); - tls[TLS_SLOT_STACK_GUARD] = __generate_stack_chk_guard(); } diff --git a/libc/bionic/ssp.cpp b/libc/bionic/ssp.cpp index 08c36c54e..f01fee6bd 100644 --- a/libc/bionic/ssp.cpp +++ b/libc/bionic/ssp.cpp @@ -32,15 +32,17 @@ #include #include #include +#include #include #include "bionic_ssp.h" #include "logd.h" -void* __stack_chk_guard = NULL; +uintptr_t __stack_chk_guard = NULL; static void __attribute__((constructor)) __init_stack_check_guard() { - __stack_chk_guard = __generate_stack_chk_guard(); + // AT_RANDOM is a pointer to 16 bytes of randomness on the stack. + __stack_chk_guard = *reinterpret_cast(getauxval(AT_RANDOM)); } // This is the crash handler. diff --git a/libc/private/bionic_ssp.h b/libc/private/bionic_ssp.h index 697216c18..d34b6ab6a 100644 --- a/libc/private/bionic_ssp.h +++ b/libc/private/bionic_ssp.h @@ -29,48 +29,14 @@ #ifndef _PRIVATE_SSP_H #define _PRIVATE_SSP_H -#include -#include - __BEGIN_DECLS -/** WARNING WARNING WARNING - ** - ** This header file is *NOT* part of the public Bionic ABI/API - ** and should not be used/included by user-serviceable parts of - ** the system (e.g. applications). - **/ - -/* GCC uses this on ARM and MIPS. */ -extern void* __stack_chk_guard; +/* GCC uses this on ARM and MIPS; we use it on x86 to set the guard in TLS. */ +extern uintptr_t __stack_chk_guard; /* GCC calls this if a stack guard check fails. */ extern void __stack_chk_fail(); -__inline__ static void* __attribute__((always_inline)) __generate_stack_chk_guard(void) { - union { - uintptr_t value; - char bytes[sizeof(uintptr_t)]; - } u; - - /* Try pulling random bytes from /dev/urandom. */ - int fd = TEMP_FAILURE_RETRY(open("/dev/urandom", O_RDONLY)); - if (fd != -1) { - ssize_t byte_count = TEMP_FAILURE_RETRY(read(fd, &u.bytes, sizeof(u))); - close(fd); - if (byte_count == sizeof(u)) { - return (void*) u.value; - } - } - - /* If that failed, switch to 'terminator canary'. */ - u.bytes[0] = 0; - u.bytes[1] = 0; - u.bytes[2] = '\n'; - u.bytes[3] = 255; - return (void*) u.value; -} - __END_DECLS #endif diff --git a/libc/private/bionic_tls.h b/libc/private/bionic_tls.h index 920f5067b..b983fbc95 100644 --- a/libc/private/bionic_tls.h +++ b/libc/private/bionic_tls.h @@ -133,9 +133,11 @@ extern void* __get_tls( void ); /* return the stack base and size, used by our malloc debugger */ extern void* __get_stack_base(int *p_stack_size); -/* Initialize the TLS. */ -extern void __libc_init_tls(void* kernel_argument_block); - __END_DECLS +#if defined(__cplusplus) +struct KernelArgumentBlock; +extern void __libc_init_tls(KernelArgumentBlock& args); +#endif + #endif /* _SYS_TLS_H */ diff --git a/linker/linker.cpp b/linker/linker.cpp index 77c29a1d6..d36b1f6b1 100755 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -1781,7 +1781,7 @@ static unsigned __linker_init_post_relocation(KernelArgumentBlock& args, unsigne * to point to a different location to ensure that no other * shared library constructor can access it. */ - __libc_init_tls(&args); + __libc_init_tls(args); #if TIMING struct timeval t0, t1; diff --git a/tests/stack_protector_test.cpp b/tests/stack_protector_test.cpp index 9cf3c38ab..664a11e7b 100644 --- a/tests/stack_protector_test.cpp +++ b/tests/stack_protector_test.cpp @@ -56,13 +56,7 @@ struct stack_protector_checker { // Duplicate tid. gettid(2) bug? Seeing this would be very upsetting. ASSERT_TRUE(tids.find(tid) == tids.end()); -#ifdef __GLIBC__ - // glibc uses the same guard for every thread. bionic uses a different guard for each one. -#else - // Duplicate guard. Our bug. Note this is potentially flaky; we _could_ get the - // same guard for two threads, but it should be vanishingly unlikely. - ASSERT_TRUE(guards.find(guard) == guards.end()); -#endif + // Uninitialized guard. Our bug. Note this is potentially flaky; we _could_ get // four random zero bytes, but it should be vanishingly unlikely. ASSERT_NE(guard, 0U); @@ -78,7 +72,7 @@ static void* ThreadGuardHelper(void* arg) { return NULL; } -TEST(stack_protector, guard_per_thread) { +TEST(stack_protector, same_guard_per_thread) { stack_protector_checker checker; size_t thread_count = 10; for (size_t i = 0; i < thread_count; ++i) { @@ -90,12 +84,8 @@ TEST(stack_protector, guard_per_thread) { } ASSERT_EQ(thread_count, checker.tids.size()); - // glibc uses the same guard for every thread. bionic uses a different guard for each one. -#ifdef __BIONIC__ - ASSERT_EQ(thread_count, checker.guards.size()); -#else + // bionic and glibc use the same guard for every thread. ASSERT_EQ(1U, checker.guards.size()); -#endif } #endif @@ -107,11 +97,11 @@ TEST(stack_protector, guard_per_thread) { // Bionic has the global for x86 too, to support binaries that can run on // Android releases that didn't implement the TLS guard value. -extern "C" void* __stack_chk_guard; +extern "C" uintptr_t __stack_chk_guard; TEST(stack_protector, global_guard) { ASSERT_NE(0, gettid()); - ASSERT_NE(0U, reinterpret_cast(__stack_chk_guard)); + ASSERT_NE(0U, __stack_chk_guard); } /* @@ -124,7 +114,7 @@ TEST(stack_protector, global_guard) { */ __attribute__ ((noinline)) static void do_modify_stack_chk_guard() { - __stack_chk_guard = (void *) 0x12345678; + __stack_chk_guard = 0x12345678; } // We have to say "DeathTest" here so gtest knows to run this test (which exits)