Per-thread -fstack-protector guards for x86.

Based on a pair of patches from Intel:

  https://android-review.googlesource.com/#/c/43909/
  https://android-review.googlesource.com/#/c/44903/

For x86, this patch supports _both_ the global that ARM/MIPS use
and the per-thread TLS entry (%gs:20) that GCC uses by default. This
lets us support binaries built with any x86 toolchain (right now,
the NDK is emitting x86 code that uses the global).

I've also extended the original tests to cover ARM/MIPS too, and
be a little more thorough for x86.

Change-Id: I02f279a80c6b626aecad449771dec91df235ad01

4 files changed, 133 insertions, 4 deletions

diff --git a/tests/Android.mk b/tests/Android.mk
index 259aced..e38aaf9 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -18,10 +18,17 @@ ifneq ($(BUILD_TINY_ANDROID), true)
 
 LOCAL_PATH := $(call my-dir)
 
+test_c_flags = \
+    -fstack-protector \
+    -g \
+    -Wall -Wextra \
+    -Werror \
+
 test_src_files = \
     getcwd_test.cpp \
     pthread_test.cpp \
     regex_test.cpp \
+    stack_protector_test.cpp \
     stdio_test.cpp \
     stdlib_test.cpp \
     string_test.cpp \
@@ -36,6 +43,7 @@ test_dynamic_src_files = \
 include $(CLEAR_VARS)
 LOCAL_MODULE := bionic-unit-tests
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_CFLAGS += $(test_c_flags)
 LOCAL_LDFLAGS += $(test_dynamic_ldflags)
 LOCAL_SHARED_LIBRARIES += libdl
 LOCAL_SRC_FILES := $(test_src_files) $(test_dynamic_src_files)
@@ -46,6 +54,7 @@ include $(BUILD_NATIVE_TEST)
 include $(CLEAR_VARS)
 LOCAL_MODULE := bionic-unit-tests-static
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_CFLAGS += $(test_c_flags)
 LOCAL_FORCE_STATIC_EXECUTABLE := true
 LOCAL_SRC_FILES := $(test_src_files)
 LOCAL_STATIC_LIBRARIES += libstlport_static libstdc++ libm libc
@@ -59,6 +68,7 @@ ifeq ($(HOST_OS)-$(HOST_ARCH),linux-x86)
 include $(CLEAR_VARS)
 LOCAL_MODULE := bionic-unit-tests-glibc
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_CFLAGS += $(test_c_flags)
 LOCAL_LDFLAGS += -lpthread -ldl
 LOCAL_LDFLAGS += $(test_dynamic_ldflags)
 LOCAL_SRC_FILES := $(test_src_files) $(test_dynamic_src_files)
diff --git a/tests/dlopen_test.cpp b/tests/dlopen_test.cpp
index 5b5c7f6..d38d8c5 100644
--- a/tests/dlopen_test.cpp
+++ b/tests/dlopen_test.cpp
@@ -58,7 +58,7 @@ TEST(dlopen, dlopen_failure) {
 #endif
 }
 
-static void* ConcurrentDlErrorFn(void* arg) {
+static void* ConcurrentDlErrorFn(void*) {
   dlopen("/child/thread", RTLD_NOW);
   return reinterpret_cast<void*>(strdup(dlerror()));
 }
diff --git a/tests/stack_protector_test.cpp b/tests/stack_protector_test.cpp
new file mode 100644
index 0000000..9d86506
--- /dev/null
+++ b/tests/stack_protector_test.cpp
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * Contributed by: Intel Corporation
+ */
+
+#include <gtest/gtest.h>
+
+#include <pthread.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <set>
+
+#ifdef __GLIBC__
+
+// glibc doesn't expose gettid(2).
+pid_t gettid() { return syscall(__NR_gettid); }
+
+#endif
+
+#ifdef __i386__
+
+// For x86, bionic and glibc have per-thread stack guard values.
+
+static uint32_t GetGuardFromTls() {
+  uint32_t guard;
+  asm ("mov %%gs:0x14, %0": "=d" (guard));
+  return guard;
+}
+
+struct stack_protector_checker {
+  std::set<pid_t> tids;
+  std::set<uint32_t> guards;
+
+  void Check() {
+    pid_t tid = gettid();
+    uint32_t guard = GetGuardFromTls();
+
+    printf("[thread %d] %%gs:0x14 = 0x%08x\n", tid, guard);
+
+    // 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);
+
+    tids.insert(tid);
+    guards.insert(guard);
+  }
+};
+
+static void* ThreadGuardHelper(void* arg) {
+  stack_protector_checker* checker = reinterpret_cast<stack_protector_checker*>(arg);
+  checker->Check();
+  return NULL;
+}
+
+TEST(stack_protector, guard_per_thread) {
+  stack_protector_checker checker;
+  size_t thread_count = 10;
+  for (size_t i = 0; i < thread_count; ++i) {
+    pthread_t t;
+    ASSERT_EQ(0, pthread_create(&t, NULL, ThreadGuardHelper, &checker));
+    void* result;
+    ASSERT_EQ(0, pthread_join(t, &result));
+    ASSERT_EQ(NULL, result);
+  }
+  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
+  ASSERT_EQ(1U, checker.guards.size());
+#endif
+}
+
+#endif
+
+#if defined(__BIONIC__) || defined(__arm__) || defined(__mips__)
+
+// For ARM and MIPS, glibc has a global stack check guard value.
+
+// 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;
+
+TEST(stack_protector, global_guard) {
+  ASSERT_NE(0, gettid());
+  ASSERT_NE(0U, reinterpret_cast<uintptr_t>(__stack_chk_guard));
+}
+
+#endif
diff --git a/tests/string_test.cpp b/tests/string_test.cpp
index 47469d8..472aacb 100644
--- a/tests/string_test.cpp
+++ b/tests/string_test.cpp
@@ -29,12 +29,13 @@ TEST(string, strerror) {
   ASSERT_STREQ("Unknown error 1234", strerror(1234));
 }
 
-static void* ConcurrentStrErrorFn(void* arg) {
+#if __BIONIC__ // glibc's strerror isn't thread safe, only its strsignal.
+
+static void* ConcurrentStrErrorFn(void*) {
   bool equal = (strcmp("Unknown error 2002", strerror(2002)) == 0);
   return reinterpret_cast<void*>(equal);
 }
 
-#if __BIONIC__ // glibc's strerror isn't thread safe, only its strsignal.
 TEST(string, strerror_concurrent) {
   const char* strerror1001 = strerror(1001);
   ASSERT_STREQ("Unknown error 1001", strerror1001);
@@ -47,6 +48,7 @@ TEST(string, strerror_concurrent) {
 
   ASSERT_STREQ("Unknown error 1001", strerror1001);
 }
+
 #endif
 
 #if __BIONIC__ // glibc's strerror_r doesn't even have the same signature as the POSIX one.
@@ -88,7 +90,7 @@ TEST(string, strsignal) {
   ASSERT_STREQ("Unknown signal 1234", strsignal(1234)); // Too large.
 }
 
-static void* ConcurrentStrSignalFn(void* arg) {
+static void* ConcurrentStrSignalFn(void*) {
   bool equal = (strcmp("Unknown signal 2002", strsignal(2002)) == 0);
   return reinterpret_cast<void*>(equal);
 }