Switch to using AT_RANDOM for the stack guards.

Bug: 7959813
Change-Id: I8db4b8912ba649bfe668c6f22aa44690ddd401a2

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 <unistd.h>
+// Declared in <unistd.h>.
 char** environ;
 
 // Declared in <asm/page.h>.
@@ -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) {

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();

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();
 }
 
 

libc/bionic/ssp.cpp

@@ -32,15 +32,17 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/auxv.h>
 #include <unistd.h>
 
 #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<uintptr_t*>(getauxval(AT_RANDOM));
 }
 
 // This is the crash handler.

libc/private/bionic_ssp.h

@@ -29,48 +29,14 @@
 #ifndef _PRIVATE_SSP_H
 #define _PRIVATE_SSP_H
 
-#include <errno.h>
-#include <sys/cdefs.h>
-
 __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

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 */

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;

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<uintptr_t>(__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)