[wasm] Initial signal handler

src/trap-handler/trap-handler.cc

+// 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 general support code for trap handling, and is mostly
+// used by other parts of V8. For the code that runs in the signal handler
+// itself, see signal-handler.cc.
+
+#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(...)
+
+namespace v8 {
+namespace internal {
+namespace trap_handler {
+
+THREAD_LOCAL bool g_thread_in_wasm_code = false;
+
+const size_t kInitialCodeObjectSize = 1024;
+const size_t kCodeObjectGrowthFactor = 2;
+size_t gNumCodeObjects = 0;
+CodeProtectionInfo **gCodeObjects = nullptr;
+
+std::atomic_flag MetadataLock::spinlock_ = ATOMIC_FLAG_INIT;
+
+constexpr size_t HandlerDataSize(size_t num_protected_instructions) {
+  return sizeof(CodeProtectionInfo) +

[Mark Seaborn, 2017/01/24 07:14:39]

I think this should be offsetof(CodeProtectionInfo, instructions).  Otherwise,
isn't it larger than necessary by one element?

[Eric Holk, 2017/01/26 01:33:37]

Done.

+         num_protected_instructions * sizeof(ProtectedInstructionData);
+}
+
+CodeProtectionInfo *CreateHandlerData(
+    void *base, size_t size, size_t num_protected_instructions,
+    ProtectedInstructionData *protected_instructions) {
+  const size_t alloc_size = HandlerDataSize(num_protected_instructions);
+  CodeProtectionInfo *data =
+      reinterpret_cast<CodeProtectionInfo *>(malloc(alloc_size));
+
+  if (data == nullptr) {
+    return nullptr;
+  }
+
+  data->base = base;
+  data->size = size;
+  data->num_protected_instructions = num_protected_instructions;
+
+  memcpy(data->instructions, protected_instructions,
+         num_protected_instructions * sizeof(ProtectedInstructionData));
+
+  return data;
+}
+
+void UpdateCodePointer(int index, void *base) {
+  MetadataLock lock;
+  CodeProtectionInfo *data = gCodeObjects[index];
+  data->base = base;
+}
+
+int RegisterHandlerData(void *base, size_t size,
+                        size_t num_protected_instructions,
+                        ProtectedInstructionData *protected_instructions) {
+  // TODO(eholk): in debug builds, make sure this data isn't already registered.
+
+  CodeProtectionInfo *data = CreateHandlerData(
+      base, size, num_protected_instructions, protected_instructions);
+
+  MetadataLock lock;
+
+  size_t i;
+  for (i = 0; i < gNumCodeObjects; ++i) {
+    if (gCodeObjects[i] == nullptr) {
+      break;
+    }
+  }
+
+  // We didn't find an opening in the available space, so grow.
+  if (i == gNumCodeObjects) {
+    size_t new_size = gNumCodeObjects > 0
+                          ? gNumCodeObjects * kCodeObjectGrowthFactor
+                          : kInitialCodeObjectSize;
+    gCodeObjects = static_cast<CodeProtectionInfo **>(
+        realloc(gCodeObjects, sizeof(*gCodeObjects) * new_size));
+
+    if (gCodeObjects == nullptr) {

[Mark Seaborn, 2017/01/24 07:14:40]

If this happens, then we've just set gCodeObjects to nullptr but gNumCodeObjects
may still be non-zero. :-)

Does V8 follow Chromium's policy to abort on the failure of small allocations?

[Eric Holk, 2017/01/26 01:33:37]

Yikes! Good catch.
Good question. I think the sensible thing to do here is to abort because
whatever we previously there is now gone, so there is no way to recover from
Wasm faults. They're should be still safe to run, but now V8 will crash on an
out of bounds access in Wasm instead of recovering from it.

Done.

+      return -1;
+    }
+
+    memset(gCodeObjects + gNumCodeObjects, 0,
+           sizeof(*gCodeObjects) * (new_size - gNumCodeObjects));
+    gNumCodeObjects = new_size;
+  }
+
+  gCodeObjects[i] = data;
+  TRACE("Registered handler data %p at index %d\n", data, i);
+  return static_cast<int>(i);

[Mark Seaborn, 2017/01/24 07:14:39]

It's probably safer to make the return type a size_t, in case an attacker can
allocate 2^31 code blocks.

[Eric Holk, 2017/01/26 01:33:37]

Good point.

At the moment, this immediately gets stored in a smaller integer (Smi), which is
limited to 31 bits.

What if instead we find an open slot and fail (return -1) if its index is more
than 2^31?

For what it's worth, I'm considering changing this protocol because right now
the index is stored on a field in the Code object, which means we increase the
memory usage measurably on even non-wasm Code objects.

+}
+
+void ReleaseHandlerData(int index) {
+  // Remove the data from the global list if it's there.
+
+  MetadataLock lock;
+
+  CodeProtectionInfo *data = gCodeObjects[index];
+  gCodeObjects[index] = nullptr;
+
+  TRACE("Released handler data %p at index %d\n", data, index);
+
+  // TODO(eholk): on debug builds, ensure there are no more copies in
+  // the list.
+  free(data);
+}
+
+void SetThreadInWasm() {
+  TRACE("Transitioning to Wasm\n");
+  g_thread_in_wasm_code = true;
+}
+
+void ClearThreadInWasm() {
+  TRACE("Transitioning from Wasm\n");
+  g_thread_in_wasm_code = false;
+}
+
+bool IsThreadInWasm() { return g_thread_in_wasm_code; }
+
+bool EnableTrapHandler() {
+  return FLAG_wasm_trap_handler && kTrapHandlerSupported;
+}
+
+}  // namespace trap_handler
+}  // namespace internal
+}  // namespace v8