[wasm] Initial signal handler

src/trap-handler/signal-handler.cc

Index: src/trap-handler/signal-handler.cc
diff --git a/src/trap-handler/signal-handler.cc b/src/trap-handler/signal-handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f6ccdc8970a9bf944e9371764c09ddef1182843f
--- /dev/null
+++ b/src/trap-handler/signal-handler.cc
@@ -0,0 +1,213 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// PLEASE READ BEFORE CHANGING THIS FILE!
+//
+// This file implements the out of bounds signal handler for
+// WebAssembly. Signal handlers are notoriously difficult to get
+// right, and getting it wrong can lead to security
+// vulnerabilities. In order to minimize this risk, here are some
+// rules to follow.
+//
+// 1. Do not introduce any new external dependencies. This file needs
+//    to be self contained so it is easy to audit everything that a
+//    signal handler might do.
+//
+// 2. Any changes must be reviewed by someone from the crash reporting
+//    or security team.
+
+// This file contains all of the code that actually runs in a signal handler
+// context.
+
+#include <signal.h>
+#include <stddef.h>
+#include <stdio.h>

[Mark Seaborn, 2017/02/09 16:44:36]

If we're following our rules about minimising dependencies, we should remove
string.h (unless we need memcpy()) and stdio.h.

[Eric Holk, 2017/02/15 02:02:44]

Done.

+#include <stdlib.h>
+#include <string.h>
+
+#include <atomic>
+
+#include "src/flags.h"

[Mark Seaborn, 2017/02/09 16:44:35]

We shouldn't have a dependency on that, right?  It's for command line flags.

[Eric Holk, 2017/02/15 02:02:44]

Done.

+#include "src/trap-handler/trap-handler-internal.h"
+#include "src/trap-handler/trap-handler.h"
+
+#define TRACE(...)

[Mark Seaborn, 2017/02/09 16:44:36]

What is V8's policy on leaving debug logging in code?

[Eric Holk, 2017/02/15 02:02:44]

I'm not sure about V8's in specific, but the chromium policy is to not do it, so
I'll remove it.

Done.

+
+namespace v8 {
+namespace internal {
+namespace trap_handler {
+
+MetadataLock::MetadataLock() {
+  if (g_thread_in_wasm_code) {
+    abort();
+  }
+
+  while (spinlock_.test_and_set(std::memory_order::memory_order_acquire)) {
+  }
+}
+
+MetadataLock::~MetadataLock() {
+  if (g_thread_in_wasm_code) {
+    abort();
+  }
+
+  spinlock_.clear(std::memory_order::memory_order_release);
+}
+
+namespace {
+
+#if V8_TRAP_HANDLER_SUPPORTED
+class SigUnmaskStack {
+ public:
+  explicit SigUnmaskStack(sigset_t sigs) {
+    pthread_sigmask(SIG_UNBLOCK, &sigs, &old_mask_);
+  }
+
+  ~SigUnmaskStack() {
+    // TODO(eholk): consider using linux-syscall-support instead of

[Mark Seaborn, 2017/01/27 20:17:30]

Nit: This comment should probably go on the first call to pthread_sigmask().

[Eric Holk, 2017/02/02 18:43:30]

Done.

+    // pthread_sigmask.
+    pthread_sigmask(SIG_SETMASK, &old_mask_, nullptr);
+  }
+
+ private:
+  sigset_t old_mask_;
+
+  SigUnmaskStack(const SigUnmaskStack&) = delete;

[Mark Seaborn, 2017/02/09 16:44:36]

You could comment this with: We'd usually use DISALLOW_COPY_AND_ASSIGN, but we
are avoiding a dependency on base/macros.h.

[Eric Holk, 2017/02/15 02:02:44]

Done.

+  void operator=(const SigUnmaskStack&) = delete;
+};
+#endif
+}  // namespace
+
+bool TryHandleFault(void* source_instruction, void* target_data,
+                    void** return_address) {
+#if V8_TRAP_HANDLER_SUPPORTED
+  // Ensure the faulting thread was actually running Wasm code.
+  if (!g_thread_in_wasm_code) {
+    TRACE("signal handler: Thread not running WASM code; crashing\n");
+    return false;
+  }
+
+  // Clear g_thread_in_wasm_code, primarily to protect against nested faults.
+  g_thread_in_wasm_code = false;
+
+  // Unmask the signal so that if this signal handler crashes, the crash will be
+  // handled by the crash reporter.  Otherwise, the process might be killed with
+  // the crash going unreported.
+  sigset_t sigs;
+  // Fortunately, sigemptyset and sigaddset are async-signal-safe according to
+  // the POSIX standard.
+  sigemptyset(&sigs);
+  sigaddset(&sigs, SIGSEGV);
+  sigaddset(&sigs, SIGBUS);
+  SigUnmaskStack unmask(sigs);
+
+  uintptr_t fault_addr = reinterpret_cast<uintptr_t>(source_instruction);
+
+  TRACE("Handling fault at %p\n", reinterpret_cast<void*>(fault_addr));
+
+  // TODO(eholk): broad code range check
+
+  MetadataLock lock_holder;
+
+  for (size_t i = 0; i < gNumCodeObjects; ++i) {
+    const CodeProtectionInfo* data = gCodeObjects[i];
+    if (data == nullptr) {
+      continue;
+    }
+    const uintptr_t base = reinterpret_cast<uintptr_t>(data->base);
+
+    TRACE("  Checking range base=%p, length = %zu\n",
+          reinterpret_cast<void*>(base), data->size);
+
+    if (fault_addr >= base && fault_addr < base + data->size) {
+      // Hurray, we found the code object. Check for protected addresses.
+      const ptrdiff_t offset = fault_addr - base;
+
+      TRACE("Range found, offset is %td\n", offset);
+
+      for (unsigned i = 0; i < data->num_protected_instructions; ++i) {
+        TRACE("  Checking instr offset %d, landing offset %d\n",
+              data->instructions[i].instr_offset,
+              data->instructions[i].landing_offset);
+        if (data->instructions[i].instr_offset == offset) {
+          // Hurray again, we found the actual instruction. Jump to
+          // the landing pad.
+
+          *return_address = reinterpret_cast<void*>(
+              data->instructions[i].landing_offset + base);
+          return true;
+        }
+      }
+    }
+  }
+
+  // If we get here, it's not a recoverable wasm fault, so we go to the next
+  // handler.
+  g_thread_in_wasm_code = true;
+  return false;
+#else
+  abort();
+#endif
+}
+
+namespace {
+
+#if V8_TRAP_HANDLER_SUPPORTED
+void HandleSignal(int signum, siginfo_t* info, void* context) {
+  // Bail out early in case we got called for the wrong kind of signal.
+  if (signum != SIGSEGV && signum != SIGBUS) {
+    return;
+  }
+
+  // Make sure the signal was generated by the kernel and not some other source.
+  if (info->si_code <= 0 || info->si_code == SI_USER ||
+      info->si_code == SI_QUEUE || info->si_code == SI_TIMER ||
+      info->si_code == SI_ASYNCIO || info->si_code == SI_MESGQ) {
+    return;
+  }
+
+  ucontext_t* uc = reinterpret_cast<ucontext_t*>(context);
+
+  void* source_instruction =
+      reinterpret_cast<void*>(uc->uc_mcontext.gregs[REG_RIP]);
+  void* target_data = reinterpret_cast<void*>(info->si_addr);
+
+  void* return_address;
+
+  if (!TryHandleFault(source_instruction, target_data, &return_address)) {
+    abort();
+  }
+
+  uc->uc_mcontext.gregs[REG_RIP] = reinterpret_cast<intptr_t>(return_address);
+}
+#endif
+}  // namespace
+
+bool RegisterDefaultSignalHandler() {

[Mark Seaborn, 2017/02/09 16:44:36]

If we're restricting this file to code that runs inside the signal handler, then
this ought to go in the other .cc file.

[Eric Holk, 2017/02/15 02:02:44]

Done.

+#if V8_TRAP_HANDLER_SUPPORTED
+  struct sigaction action;
+  struct sigaction old_action;
+  action.sa_sigaction = HandleSignal;
+  action.sa_flags = SA_SIGINFO;
+  sigemptyset(&action.sa_mask);
+  if (sigaction(SIGSEGV, &action, &old_action) != 0) {
+    return false;
+  }
+
+  if (sigaction(SIGBUS, &action, nullptr) != 0) {
+    // Undo registering for SIGSEGV so that this function has no effect if

[Mark Seaborn, 2017/01/27 20:17:30]

The caller doesn't actually check for an error.  Can you either make this abort,
or have the caller check?

[Eric Holk, 2017/02/02 18:43:30]

I made the caller check.

Done.

+    // registering for either SIGSEGV or SIGBUS fail.
+    sigaction(SIGSEGV, &old_action, nullptr);
+    return false;
+  }
+
+  return true;
+#else
+  return false;
+#endif
+}
+
+}  // namespace trap_handler
+}  // namespace internal
+}  // namespace v8