Skip to content

Commit

Permalink
src: handle wasm out of bound in osx will raise SIGBUS correctly
Browse files Browse the repository at this point in the history
fix: #46559
OSX will raise both SIGBUS and SIGSEGV when out of bound memory visit,
This commit set sigaction in OSX for two signals to handle this.

PR-URL: #46561
Fixes: #46559
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
  • Loading branch information
HerrCai0907 authored and ruyadorno committed Sep 12, 2023
1 parent 7051caf commit 6c72622
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
24 changes: 23 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,19 @@ static LONG TrapWebAssemblyOrContinue(EXCEPTION_POINTERS* exception) {
}
#else
static std::atomic<sigaction_cb> previous_sigsegv_action;
// TODO(align behavior between macos and other in next major version)
#if defined(__APPLE__)
static std::atomic<sigaction_cb> previous_sigbus_action;
#endif // __APPLE__

void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) {
if (!v8::TryHandleWebAssemblyTrapPosix(signo, info, ucontext)) {
#if defined(__APPLE__)
sigaction_cb prev = signo == SIGBUS ? previous_sigbus_action.load()
: previous_sigsegv_action.load();
#else
sigaction_cb prev = previous_sigsegv_action.load();
#endif // __APPLE__
if (prev != nullptr) {
prev(signo, info, ucontext);
} else {
Expand Down Expand Up @@ -426,6 +435,15 @@ void RegisterSignalHandler(int signal,
previous_sigsegv_action.store(handler);
return;
}
// TODO(align behavior between macos and other in next major version)
#if defined(__APPLE__)
if (signal == SIGBUS) {
CHECK(previous_sigbus_action.is_lock_free());
CHECK(!reset_handler);
previous_sigbus_action.store(handler);
return;
}
#endif // __APPLE__
#endif // NODE_USE_V8_WASM_TRAP_HANDLER
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
Expand Down Expand Up @@ -576,14 +594,18 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) {
#else
// Tell V8 to disable emitting WebAssembly
// memory bounds checks. This means that we have
// to catch the SIGSEGV in TrapWebAssemblyOrContinue
// to catch the SIGSEGV/SIGBUS in TrapWebAssemblyOrContinue
// and pass the signal context to V8.
{
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_sigaction = TrapWebAssemblyOrContinue;
sa.sa_flags = SA_SIGINFO;
CHECK_EQ(sigaction(SIGSEGV, &sa, nullptr), 0);
// TODO(align behavior between macos and other in next major version)
#if defined(__APPLE__)
CHECK_EQ(sigaction(SIGBUS, &sa, nullptr), 0);
#endif
}
#endif // defined(_WIN32)
V8::EnableWebAssemblyTrapHandler(false);
Expand Down
Binary file added test/fixtures/out-of-bound.wasm
Binary file not shown.
16 changes: 16 additions & 0 deletions test/fixtures/out-of-bound.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
(module
(type $none_=>_none (func))
(memory $0 1)
(export "_start" (func $_start))
(func $_start
memory.size
i32.const 64
i32.mul
i32.const 1024
i32.mul
i32.const 3
i32.sub
i32.load
drop
)
)
12 changes: 12 additions & 0 deletions test/parallel/test-wasm-memory-out-of-bound.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fixtures = require('../common/fixtures');

const buffer = fixtures.readSync('out-of-bound.wasm');
WebAssembly.instantiate(buffer, {}).then(common.mustCall((results) => {
assert.throws(() => {
results.instance.exports._start();
}, WebAssembly.RuntimeError('memory access out of bounds'));
}));

0 comments on commit 6c72622

Please sign in to comment.