Merge "Per-thread -fstack-protector guards for x86."

libc/Android.mk

@@ -740,7 +740,7 @@ WITH_MALLOC_CHECK_LIBC_A := $(strip $(WITH_MALLOC_CHECK_LIBC_A))
 
 include $(CLEAR_VARS)
 
-LOCAL_SRC_FILES := bionic/ssp.c
+LOCAL_SRC_FILES := bionic/ssp.cpp
 LOCAL_CFLAGS := $(libc_common_cflags) -fno-stack-protector
 LOCAL_C_INCLUDES := $(libc_common_c_includes)
 LOCAL_MODULE := libbionic_ssp

libc/bionic/pthread.c

@@ -48,6 +48,7 @@
 #include "bionic_atomic_inline.h"
 #include "bionic_futex.h"
 #include "bionic_pthread.h"
+#include "bionic_ssp.h"
 #include "bionic_tls.h"
 #include "pthread_internal.h"
 #include "thread_private.h"
@@ -171,12 +172,14 @@ void  __init_tls(void** tls, void* thread) {
     tls[i] = NULL;
   }
 
-  // Slot 0 must point to the tls area, this is required by the implementation
+  // Slot 0 must point to itself. The x86 Linux kernel reads the TLS from %fs:0.
-  // of the x86 Linux kernel thread-local-storage.
   tls[TLS_SLOT_SELF]      = (void*) tls;
   tls[TLS_SLOT_THREAD_ID] = thread;
 
+  // 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.c

@@ -1,100 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *  * Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *  * Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-#include <stdio.h>
-#include <stdlib.h>
-#include <signal.h>
-#include <string.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <ctype.h>
-#include "logd.h"
-
-void *__stack_chk_guard = 0;
-
-/* Initialize the canary with a random value from /dev/urandom.
- * If that fails, use the "terminator canary". */
-static void __attribute__ ((constructor))
-__guard_setup(void)
-{
-    int fd;
-
-    fd = open("/dev/urandom", O_RDONLY);
-    if (fd != -1) {
-        ssize_t len = read(fd, &__stack_chk_guard,
-                           sizeof(__stack_chk_guard));
-        close(fd);
-        if (len == sizeof(__stack_chk_guard))
-            return;
-    }
-
-    /* If that failed, switch to 'terminator canary' */
-    ((unsigned char *)&__stack_chk_guard)[0] = 0;
-    ((unsigned char *)&__stack_chk_guard)[1] = 0;
-    ((unsigned char *)&__stack_chk_guard)[2] = '\n';
-    ((unsigned char *)&__stack_chk_guard)[3] = 255;
-}
-
-/* This is the crash handler.
- * Does a best effort at logging and calls _exit to terminate
- * the process immediately (without atexit handlers, etc.) */
-void __stack_chk_fail(void)
-{
-    struct sigaction sa;
-    sigset_t sigmask;
-    static const char message[] = "stack corruption detected: aborted";
-    char path[PATH_MAX];
-    int count;
-
-    /* Immediately block all (but SIGABRT) signal handlers from running code */
-    sigfillset(&sigmask);
-    sigdelset(&sigmask, SIGABRT);
-    sigprocmask(SIG_BLOCK, &sigmask, NULL);
-
-    /* Use /proc/self/exe link to obtain the program name for logging
-     * purposes. If it's not available, we set it to "<unknown>" */
-    if ((count = readlink("/proc/self/exe", path, sizeof(path) - 1)) == -1) {
-        strlcpy(path, "<unknown>", sizeof(path));
-    } else {
-        path[count] = '\0';
-    }
-
-    /* Do a best effort at logging. This ends up calling writev(2) */
-    __libc_android_log_print(ANDROID_LOG_FATAL, path, message);
-
-    /* Make sure there is no default action for SIGABRT */
-    bzero(&sa, sizeof(struct sigaction));
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = 0;
-    sa.sa_handler = SIG_DFL;
-    sigaction(SIGABRT, &sa, NULL);
-
-    /* Terminate the process and exit immediately */
-    kill(getpid(), SIGABRT);
-
-    _exit(127);
-}

libc/bionic/ssp.cpp

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2008 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <ctype.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "bionic_ssp.h"
+#include "logd.h"
+
+void* __stack_chk_guard = NULL;
+
+static void __attribute__((constructor)) __init_stack_check_guard() {
+  __stack_chk_guard = __generate_stack_chk_guard();
+}
+
+// This is the crash handler.
+// Does a best effort at logging and calls _exit to terminate
+// the process immediately (without atexit handlers, etc.).
+void __stack_chk_fail() {
+  // Immediately block all (but SIGABRT) signal handlers from running code.
+  sigset_t sigmask;
+  sigfillset(&sigmask);
+  sigdelset(&sigmask, SIGABRT);
+  sigprocmask(SIG_BLOCK, &sigmask, NULL);
+
+  // Use /proc/self/exe link to obtain the program name for logging
+  // purposes. If it's not available, we set it to "<unknown>".
+  char path[PATH_MAX];
+  int count;
+  if ((count = readlink("/proc/self/exe", path, sizeof(path) - 1)) == -1) {
+    strlcpy(path, "<unknown>", sizeof(path));
+  } else {
+    path[count] = '\0';
+  }
+
+  // Do a best effort at logging. This ends up calling writev(2).
+  __libc_android_log_print(ANDROID_LOG_FATAL, path, "stack corruption detected: aborted");
+
+  // Make sure there is no default action for SIGABRT.
+  struct sigaction sa;
+  memset(&sa, 0, sizeof(sa));
+  sigemptyset(&sa.sa_mask);
+  sa.sa_flags = 0;
+  sa.sa_handler = SIG_DFL;
+  sigaction(SIGABRT, &sa, NULL);
+
+  // Terminate the process and exit immediately.
+  kill(getpid(), SIGABRT);
+
+  _exit(127);
+}

libc/private/bionic_ssp.h

@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#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 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

@@ -43,24 +43,19 @@ __BEGIN_DECLS
  ** pre-allocated slot directly for performance reason).
  **/
 
-/* maximum number of elements in the TLS array */
+/* Maximum number of elements in the TLS array. */
 #define BIONIC_TLS_SLOTS            64
 
-/* note that slot 0, called TLS_SLOT_SELF must point to itself.
+/* Well-known TLS slots. What data goes in which slot is arbitrary unless otherwise noted. */
- * this is required to implement thread-local storage with the x86
+#define TLS_SLOT_SELF               0  /* The kernel requires this specific slot for x86. */
- * Linux kernel, that reads the TLS from fs:[0], where 'fs' is a
- * thread-specific segment descriptor...
- */
-
-/* Well-known TLS slots. */
-#define TLS_SLOT_SELF               0
 #define TLS_SLOT_THREAD_ID          1
 #define TLS_SLOT_ERRNO              2
 
 #define TLS_SLOT_OPENGL_API         3
 #define TLS_SLOT_OPENGL             4
 
-#define TLS_SLOT_DLERROR            5
+#define TLS_SLOT_STACK_GUARD        5  /* GCC requires this specific slot for x86. */
+#define TLS_SLOT_DLERROR            6
 
 #define TLS_SLOT_MAX_WELL_KNOWN     TLS_SLOT_DLERROR
 

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)

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

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

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