Merge "Switch to using AT_RANDOM for the stack guards."
diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp
index 71acc45..881b091 100644
--- a/libc/bionic/libc_init_common.cpp
+++ b/libc/bionic/libc_init_common.cpp
@@ -48,7 +48,7 @@
// 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 @@
* 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 @@
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 e5506d1..a6b20eb 100644
--- a/libc/bionic/libc_init_static.cpp
+++ b/libc/bionic/libc_init_static.cpp
@@ -87,9 +87,8 @@
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 f2a7ebe..88a972d 100644
--- a/libc/bionic/pthread.c
+++ b/libc/bionic/pthread.c
@@ -170,13 +170,12 @@
}
// 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 08c36c5..f01fee6 100644
--- a/libc/bionic/ssp.cpp
+++ b/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.
diff --git a/libc/private/bionic_ssp.h b/libc/private/bionic_ssp.h
index 697216c..d34b6ab 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 <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
diff --git a/libc/private/bionic_tls.h b/libc/private/bionic_tls.h
index 920f506..b983fbc 100644
--- a/libc/private/bionic_tls.h
+++ b/libc/private/bionic_tls.h
@@ -133,9 +133,11 @@
/* 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 77c29a1..d36b1f6 100755
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -1781,7 +1781,7 @@
* 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 9cf3c38..664a11e 100644
--- a/tests/stack_protector_test.cpp
+++ b/tests/stack_protector_test.cpp
@@ -56,13 +56,7 @@
// 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 @@
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 @@
}
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 @@
// 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 @@
*/
__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)
