Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(597)

Unified Diff: src/trap-handler/signal-handler.cc

Issue 2371833007: [wasm] Initial signal handler (Closed)
Patch Set: Addressing some of Jochen's feedback Created 3 years, 12 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..de9b47b372936e8e9e295e2d85e82d0fd3b256cb
--- /dev/null
+++ b/src/trap-handler/signal-handler.cc
@@ -0,0 +1,160 @@
+// 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>
+#include <stdlib.h>
+#include <string.h>
+
+#include <atomic>
+
+#include "src/flags.h"
+#include "src/trap-handler/trap-handler-internal.h"
+#include "src/trap-handler/trap-handler.h"
+
+#define TRACE(...)
+// #define TRACE(...) fprintf(stderr, __VA_ARGS__)
titzer 2017/01/09 09:26:56 Can you use the PrintF function here?
Eric Holk 2017/01/10 23:10:48 We'd have to include more v8 headers, which increa
+
+namespace v8 {
+namespace internal {
+namespace trap_handler {
+
+extern __thread bool g_thread_in_wasm_code;
+
+extern size_t gNumCodeObjects;
+extern CodeObjectData **gCodeObjects;
+
+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 {
+
+class SigUnmaskStack {
+ public:
+ explicit SigUnmaskStack(int signum) {
+ sigset_t sigs;
+ sigemptyset(&sigs);
+ sigaddset(&sigs, signum);
+ pthread_sigmask(SIG_UNBLOCK, &sigs, &old_mask_);
+ }
+
+ ~SigUnmaskStack() { pthread_sigmask(SIG_SETMASK, &old_mask_, nullptr); }
+
+ private:
+ sigset_t old_mask_;
+
+ SigUnmaskStack(const SigUnmaskStack&) = delete;
+ void operator=(const SigUnmaskStack&) = delete;
+};
+} // namespace
+
+bool MaybeHandleFault(int signum, void *siginfo, void *context) {
+ // Bail out early in case we got called for the wrong kind of signal.
+ if (signum != SIGSEGV && signum != SIGBUS) {
+ return false;
+ }
+
+ // Make sure the signal was generated by the kernel and not some other source.
+ siginfo_t *info = reinterpret_cast<siginfo_t *>(siginfo);
+ 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 false;
+ }
+
+ // 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;
+ }
+
+ // This isn't Wasm code anymore. At this point we were either crash or throw
titzer 2017/01/09 09:26:56 Not sure I understand this comment. Does this prot
Eric Holk 2017/01/10 23:10:48 Yes. I changed the comment to reflect that. I gue
+ // an exception, so we will not be running Wasm code again and there is no
+ // need to set the flag again later.
+ g_thread_in_wasm_code = false;
+
+ // Unmask the signal in case we fault in the signal hander.
+ SigUnmaskStack unmask(signum);
+
+ ucontext_t *uc = reinterpret_cast<ucontext_t *>(context);
+
+ // TODO(eholk): broad code range check
+ intptr_t fault_addr = uc->uc_mcontext.gregs[REG_RIP];
+
+ TRACE("Handling fault at %p\n", reinterpret_cast<void *>(fault_addr));
+
+ MetadataLock lock_holder;
+
+ for (size_t i = 0; i < gNumCodeObjects; ++i) {
+ const CodeObjectData *data = gCodeObjects[i];
+ if (data == nullptr) {
+ continue;
+ }
+ const intptr_t base = reinterpret_cast<intptr_t>(data->base);
+
+ TRACE(" Checking range base=%p, length = %zu\n",
+ reinterpret_cast<void *>(base), data->size);
+
+ if (fault_addr >= base &&
+ fault_addr < base + static_cast<intptr_t>(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.
+
+ uc->uc_mcontext.gregs[REG_RIP] =
+ data->instructions[i].landing_offset + base;
+ return true;
+ }
+ }
+ }
+ }
+
+ // If we get here, it's not a wasm fault, so we go to the next handler.
+ return false;
+}
+} // namespace trap_handler
+} // namespace internal
+} // namespace v8

Powered by Google App Engine
This is Rietveld 408576698