Skip to content

Commit 6c72622

Browse files
HerrCai0907ruyadorno
authored andcommitted
src: handle wasm out of bound in osx will raise SIGBUS correctly
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>
1 parent 7051caf commit 6c72622

File tree

4 files changed

+51
-1
lines changed

4 files changed

+51
-1
lines changed

src/node.cc

+23-1
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,19 @@ static LONG TrapWebAssemblyOrContinue(EXCEPTION_POINTERS* exception) {
393393
}
394394
#else
395395
static std::atomic<sigaction_cb> previous_sigsegv_action;
396+
// TODO(align behavior between macos and other in next major version)
397+
#if defined(__APPLE__)
398+
static std::atomic<sigaction_cb> previous_sigbus_action;
399+
#endif // __APPLE__
396400

397401
void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) {
398402
if (!v8::TryHandleWebAssemblyTrapPosix(signo, info, ucontext)) {
403+
#if defined(__APPLE__)
404+
sigaction_cb prev = signo == SIGBUS ? previous_sigbus_action.load()
405+
: previous_sigsegv_action.load();
406+
#else
399407
sigaction_cb prev = previous_sigsegv_action.load();
408+
#endif // __APPLE__
400409
if (prev != nullptr) {
401410
prev(signo, info, ucontext);
402411
} else {
@@ -426,6 +435,15 @@ void RegisterSignalHandler(int signal,
426435
previous_sigsegv_action.store(handler);
427436
return;
428437
}
438+
// TODO(align behavior between macos and other in next major version)
439+
#if defined(__APPLE__)
440+
if (signal == SIGBUS) {
441+
CHECK(previous_sigbus_action.is_lock_free());
442+
CHECK(!reset_handler);
443+
previous_sigbus_action.store(handler);
444+
return;
445+
}
446+
#endif // __APPLE__
429447
#endif // NODE_USE_V8_WASM_TRAP_HANDLER
430448
struct sigaction sa;
431449
memset(&sa, 0, sizeof(sa));
@@ -576,14 +594,18 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) {
576594
#else
577595
// Tell V8 to disable emitting WebAssembly
578596
// memory bounds checks. This means that we have
579-
// to catch the SIGSEGV in TrapWebAssemblyOrContinue
597+
// to catch the SIGSEGV/SIGBUS in TrapWebAssemblyOrContinue
580598
// and pass the signal context to V8.
581599
{
582600
struct sigaction sa;
583601
memset(&sa, 0, sizeof(sa));
584602
sa.sa_sigaction = TrapWebAssemblyOrContinue;
585603
sa.sa_flags = SA_SIGINFO;
586604
CHECK_EQ(sigaction(SIGSEGV, &sa, nullptr), 0);
605+
// TODO(align behavior between macos and other in next major version)
606+
#if defined(__APPLE__)
607+
CHECK_EQ(sigaction(SIGBUS, &sa, nullptr), 0);
608+
#endif
587609
}
588610
#endif // defined(_WIN32)
589611
V8::EnableWebAssemblyTrapHandler(false);

test/fixtures/out-of-bound.wasm

58 Bytes
Binary file not shown.

test/fixtures/out-of-bound.wat

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
(module
2+
(type $none_=>_none (func))
3+
(memory $0 1)
4+
(export "_start" (func $_start))
5+
(func $_start
6+
memory.size
7+
i32.const 64
8+
i32.mul
9+
i32.const 1024
10+
i32.mul
11+
i32.const 3
12+
i32.sub
13+
i32.load
14+
drop
15+
)
16+
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fixtures = require('../common/fixtures');
6+
7+
const buffer = fixtures.readSync('out-of-bound.wasm');
8+
WebAssembly.instantiate(buffer, {}).then(common.mustCall((results) => {
9+
assert.throws(() => {
10+
results.instance.exports._start();
11+
}, WebAssembly.RuntimeError('memory access out of bounds'));
12+
}));

0 commit comments

Comments
 (0)