Merge "Per-thread -fstack-protector guards for x86."
diff --git a/libc/Android.mk b/libc/Android.mk
index 780f478..a037e7a 100644
--- a/libc/Android.mk
+++ b/libc/Android.mk
@@ -740,7 +740,7 @@
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
diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c
index 719bc83..7c22b45 100644
--- a/libc/bionic/pthread.c
+++ b/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 @@
tls[i] = NULL;
}
- // Slot 0 must point to the tls area, this is required by the implementation
- // of the x86 Linux kernel thread-local-storage.
+ // 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_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();
}
diff --git a/libc/bionic/ssp.c b/libc/bionic/ssp.c
deleted file mode 100644
index f83b2a4..0000000
--- a/libc/bionic/ssp.c
+++ /dev/null
@@ -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);
-}
diff --git a/libc/bionic/ssp.cpp b/libc/bionic/ssp.cpp
new file mode 100644
index 0000000..fdf8832
--- /dev/null
+++ b/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);
+}
diff --git a/libc/private/bionic_ssp.h b/libc/private/bionic_ssp.h
new file mode 100644
index 0000000..697216c
--- /dev/null
+++ b/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
diff --git a/libc/private/bionic_tls.h b/libc/private/bionic_tls.h
index a626d21..f661ccf 100644
--- a/libc/private/bionic_tls.h
+++ b/libc/private/bionic_tls.h
@@ -43,24 +43,19 @@
** 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.
- * this is required to implement thread-local storage with the 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
+/* Well-known TLS slots. What data goes in which slot is arbitrary unless otherwise noted. */
+#define TLS_SLOT_SELF 0 /* The kernel requires this specific slot for x86. */
#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
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 @@
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 @@
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 $(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 @@
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 @@
#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 @@
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 @@
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 @@
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);
}