OSDN Git Service

Fix CFI initialization crash on x86.
authorEvgenii Stepanov <eugenis@google.com>
Tue, 31 Jan 2017 21:19:30 +0000 (13:19 -0800)
committerEvgenii Stepanov <eugenis@google.com>
Thu, 2 Feb 2017 22:44:46 +0000 (14:44 -0800)
Third try.

Bug: 34752378
Test: bionic tests
Change-Id: I247c127489a8ee38404e104f28d916a704e35f36

libdl/libdl_cfi.cpp
linker/linker_cfi.cpp
tests/cfi_test.cpp
tests/libs/Android.bp
tests/libs/cfi_test_helper.cpp [new file with mode: 0644]
tests/libs/cfi_test_helper2.cpp [new file with mode: 0644]
tests/libs/cfi_test_lib.cpp
tests/libs/libs_utils.h [new file with mode: 0644]
tests/unistd_test.cpp
tests/utils.h

index 362b093..8458564 100644 (file)
@@ -29,10 +29,12 @@ static struct {
   char padding[PAGE_SIZE - sizeof(v)];
 } shadow_base_storage alignas(PAGE_SIZE);
 
+// __cfi_init is called by the loader as soon as the shadow is mapped. This may happen very early
+// during startup, before libdl.so global constructors, and, on i386, even before __libc_sysinfo is
+// initialized. This function should not do any system calls.
 extern "C" uintptr_t* __cfi_init(uintptr_t shadow_base) {
   shadow_base_storage.v = shadow_base;
   static_assert(sizeof(shadow_base_storage) == PAGE_SIZE, "");
-  mprotect(&shadow_base_storage, PAGE_SIZE, PROT_READ);
   return &shadow_base_storage.v;
 }
 
index e9cdab6..28d2eaf 100644 (file)
@@ -193,6 +193,7 @@ bool CFIShadowWriter::NotifyLibDl(soinfo* solist, uintptr_t p) {
   shadow_start = reinterpret_cast<uintptr_t* (*)(uintptr_t)>(cfi_init)(p);
   CHECK(shadow_start != nullptr);
   CHECK(*shadow_start == p);
+  mprotect(shadow_start, PAGE_SIZE, PROT_READ);
   return true;
 }
 
index 0f93edb..5e627a7 100644 (file)
@@ -1,7 +1,26 @@
-#include <gtest/gtest.h>
+/*
+ * Copyright (C) 2017 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.
+ */
+
 #include <dlfcn.h>
+#include <gtest/gtest.h>
+#include <sys/stat.h>
 
 #include "BionicDeathTest.h"
+#include "gtest_globals.h"
+#include "utils.h"
 
 // Private libdl interface.
 extern "C" {
@@ -12,6 +31,7 @@ void __cfi_slowpath_diag(uint64_t CallSiteTypeId, void* Ptr, void* DiagData);
 static void f() {}
 
 TEST(cfi_test, basic) {
+#if defined(__BIONIC__)
   void* handle;
   handle = dlopen("libcfi-test.so", RTLD_NOW | RTLD_LOCAL);
   ASSERT_TRUE(handle != nullptr) << dlerror();
@@ -82,13 +102,39 @@ TEST(cfi_test, basic) {
   // CFI check for a function inside the unloaded DSO. This is always invalid and gets the process
   // killed.
   EXPECT_DEATH(__cfi_slowpath(45, reinterpret_cast<void*>(code_ptr)), "");
+#endif
 }
 
 TEST(cfi_test, invalid) {
+#if defined(__BIONIC__)
   void* handle;
   handle = dlopen("libcfi-test-bad.so", RTLD_NOW | RTLD_LOCAL);
   ASSERT_FALSE(handle != nullptr) << dlerror();
 
   handle = dlopen("libcfi-test-bad.so", RTLD_NOW | RTLD_LOCAL);
   ASSERT_FALSE(handle != nullptr) << dlerror();
+#endif
+}
+
+// cfi_test_helper exports __cfi_check, which triggers CFI initialization at startup.
+TEST(cfi_test, early_init) {
+#if defined(__BIONIC__)
+  std::string helper = get_testlib_root() + "/cfi_test_helper/cfi_test_helper";
+  chmod(helper.c_str(), 0755); // TODO: "x" lost in CTS, b/34945607
+  ExecTestHelper eth;
+  eth.SetArgs({ helper.c_str(), nullptr });
+  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, nullptr);
+#endif
+}
+
+// cfi_test_helper2 depends on a library that exports __cfi_check, which triggers CFI initialization
+// at startup.
+TEST(cfi_test, early_init2) {
+#if defined(__BIONIC__)
+  std::string helper = get_testlib_root() + "/cfi_test_helper2/cfi_test_helper2";
+  chmod(helper.c_str(), 0755); // TODO: "x" lost in CTS, b/34945607
+  ExecTestHelper eth;
+  eth.SetArgs({ helper.c_str(), nullptr });
+  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, nullptr);
+#endif
 }
index b5855af..7802727 100644 (file)
@@ -494,3 +494,20 @@ cc_test_library {
         cfi: false,
     },
 }
+
+cc_test {
+    name: "cfi_test_helper",
+    host_supported: false,
+    defaults: ["bionic_testlib_defaults"],
+    srcs: ["cfi_test_helper.cpp"],
+    ldflags: ["-rdynamic"],
+}
+
+cc_test {
+    name: "cfi_test_helper2",
+    host_supported: false,
+    defaults: ["bionic_testlib_defaults"],
+    srcs: ["cfi_test_helper2.cpp"],
+    shared_libs: ["libcfi-test"],
+    ldflags: ["-Wl,--rpath,${ORIGIN}/.."],
+}
diff --git a/tests/libs/cfi_test_helper.cpp b/tests/libs/cfi_test_helper.cpp
new file mode 100644 (file)
index 0000000..ff313a2
--- /dev/null
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+#include <assert.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+#include "libs_utils.h"
+
+// This library is built for all targets, including host tests, so __cfi_slowpath may not be
+// present. But it is only used in the bionic loader tests.
+extern "C" __attribute__((weak)) void __cfi_slowpath(uint64_t, void*);
+
+static int g_count;
+
+// Mock a CFI-enabled library without relying on the compiler.
+extern "C" __attribute__((aligned(4096))) void __cfi_check(uint64_t /*CallSiteTypeId*/,
+                                                           void* /*TargetAddr*/, void* /*Diag*/) {
+  ++g_count;
+}
+
+void preinit_ctor() {
+  CHECK(g_count == 0);
+  __cfi_slowpath(42, reinterpret_cast<void*>(&preinit_ctor));
+  CHECK(g_count == 1);
+}
+
+__attribute__((section(".preinit_array"), used)) void (*preinit_ctor_p)(void) = preinit_ctor;
+
+__attribute__((constructor, used)) void ctor() {
+  CHECK(g_count == 1);
+  __cfi_slowpath(42, reinterpret_cast<void*>(&ctor));
+  CHECK(g_count == 2);
+}
+
+int main(void) {
+  CHECK(g_count == 2);
+  __cfi_slowpath(42, reinterpret_cast<void*>(&main));
+  CHECK(g_count == 3);
+  return 0;
+}
diff --git a/tests/libs/cfi_test_helper2.cpp b/tests/libs/cfi_test_helper2.cpp
new file mode 100644 (file)
index 0000000..11a6036
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+#include <assert.h>
+#include <dlfcn.h>
+
+#include "libs_utils.h"
+
+int main(void) {
+  void* handle;
+  // libcfi-test.so does some basic testing in a global constructor. Check that it is linked.
+  handle = dlopen("libcfi-test.so", RTLD_NOW | RTLD_NOLOAD);
+  CHECK(handle != nullptr);
+  dlclose(handle);
+  return 0;
+}
index b0e2f42..959b102 100644 (file)
@@ -1,3 +1,19 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
 #include <assert.h>
 #include <stdint.h>
 #include <stdlib.h>
diff --git a/tests/libs/libs_utils.h b/tests/libs/libs_utils.h
new file mode 100644 (file)
index 0000000..f11cbe7
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+#ifndef LIBS_UTILS_H
+#define LIBS_UTILS_H
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#define CHECK(x)                                                            \
+  do {                                                                      \
+    fprintf(stderr, "CHECK(" #x ") failed at %s:%d\n", __FILE__, __LINE__); \
+    if (!(x)) abort();                                                      \
+  } while (0)
+
+#endif  // LIBS_UTILS_H
index 9d809f0..ddf0df5 100644 (file)
@@ -1203,52 +1203,6 @@ TEST(UNISTD_TEST, setdomainname) {
   }
 }
 
-class ExecTestHelper {
- public:
-  char** GetArgs() { return const_cast<char**>(args_.data()); }
-  char** GetEnv() { return const_cast<char**>(env_.data()); }
-
-  void SetArgs(const std::vector<const char*> args) { args_ = args; }
-  void SetEnv(const std::vector<const char*> env) { env_ = env; }
-
-  void Run(const std::function<void ()>& child_fn,
-           int expected_exit_status,
-           const char* expected_output) {
-      int fds[2];
-      ASSERT_NE(pipe2(fds, 0), -1);
-
-      pid_t pid = fork();
-      ASSERT_NE(pid, -1);
-
-      if (pid == 0) {
-          // Child.
-          close(fds[0]);
-          dup2(fds[1], STDOUT_FILENO);
-          dup2(fds[1], STDERR_FILENO);
-          if (fds[1] != STDOUT_FILENO && fds[1] != STDERR_FILENO) close(fds[1]);
-          child_fn();
-          FAIL();
-      }
-
-      // Parent.
-      close(fds[1]);
-      std::string output;
-      char buf[BUFSIZ];
-      ssize_t bytes_read;
-      while ((bytes_read = TEMP_FAILURE_RETRY(read(fds[0], buf, sizeof(buf)))) > 0) {
-          output.append(buf, bytes_read);
-      }
-      close(fds[0]);
-
-      AssertChildExited(pid, expected_exit_status);
-      ASSERT_EQ(expected_output, output);
-  }
-
- private:
-  std::vector<const char*> args_;
-  std::vector<const char*> env_;
-};
-
 #if defined(__GLIBC__)
 #define BIN_DIR "/bin/"
 #else
index 79eed10..eac6f31 100644 (file)
@@ -125,8 +125,13 @@ static inline void WaitUntilThreadSleep(std::atomic<pid_t>& tid) {
 static inline void AssertChildExited(int pid, int expected_exit_status) {
   int status;
   ASSERT_EQ(pid, waitpid(pid, &status, 0));
-  ASSERT_TRUE(WIFEXITED(status));
-  ASSERT_EQ(expected_exit_status, WEXITSTATUS(status));
+  if (expected_exit_status >= 0) {
+    ASSERT_TRUE(WIFEXITED(status));
+    ASSERT_EQ(expected_exit_status, WEXITSTATUS(status));
+  } else {
+    ASSERT_TRUE(WIFSIGNALED(status));
+    ASSERT_EQ(-expected_exit_status, WTERMSIG(status));
+  }
 }
 
 // The absolute path to the executable
@@ -142,4 +147,62 @@ int get_argc();
 char** get_argv();
 char** get_envp();
 
+// ExecTestHelper is only used in bionic and glibc tests.
+#ifndef __APPLE__
+class ExecTestHelper {
+ public:
+  char** GetArgs() {
+    return const_cast<char**>(args_.data());
+  }
+  char** GetEnv() {
+    return const_cast<char**>(env_.data());
+  }
+
+  void SetArgs(const std::vector<const char*> args) {
+    args_ = args;
+  }
+  void SetEnv(const std::vector<const char*> env) {
+    env_ = env;
+  }
+
+  void Run(const std::function<void()>& child_fn, int expected_exit_status,
+           const char* expected_output) {
+    int fds[2];
+    ASSERT_NE(pipe(fds), -1);
+
+    pid_t pid = fork();
+    ASSERT_NE(pid, -1);
+
+    if (pid == 0) {
+      // Child.
+      close(fds[0]);
+      dup2(fds[1], STDOUT_FILENO);
+      dup2(fds[1], STDERR_FILENO);
+      if (fds[1] != STDOUT_FILENO && fds[1] != STDERR_FILENO) close(fds[1]);
+      child_fn();
+      FAIL();
+    }
+
+    // Parent.
+    close(fds[1]);
+    std::string output;
+    char buf[BUFSIZ];
+    ssize_t bytes_read;
+    while ((bytes_read = TEMP_FAILURE_RETRY(read(fds[0], buf, sizeof(buf)))) > 0) {
+      output.append(buf, bytes_read);
+    }
+    close(fds[0]);
+
+    AssertChildExited(pid, expected_exit_status);
+    if (expected_output != nullptr) {
+      ASSERT_EQ(expected_output, output);
+    }
+  }
+
+ private:
+  std::vector<const char*> args_;
+  std::vector<const char*> env_;
+};
+#endif
+
 #endif