Commit 6c72622
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
4 files changed
Lines changed: 51 additions & 1 deletion
File tree
- src
- test
- fixtures
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -393,10 +393,19 @@ static LONG TrapWebAssemblyOrContinue(EXCEPTION_POINTERS* exception) { | |||
| 393 | 393 | } | |
| 394 | 394 | #else | |
| 395 | 395 | 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__ | ||
| 396 | 400 | ||
| 397 | 401 | void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) { | |
| 398 | 402 | 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 | ||
| 399 | 407 | sigaction_cb prev = previous_sigsegv_action.load(); | |
| 408 | + #endif // __APPLE__ | ||
| 400 | 409 | if (prev != nullptr) { | |
| 401 | 410 | prev(signo, info, ucontext); | |
| 402 | 411 | } else { | |
@@ -426,6 +435,15 @@ void RegisterSignalHandler(int signal, | |||
| 426 | 435 | previous_sigsegv_action.store(handler); | |
| 427 | 436 | return; | |
| 428 | 437 | } | |
| 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__ | ||
| 429 | 447 | #endif // NODE_USE_V8_WASM_TRAP_HANDLER | |
| 430 | 448 | struct sigaction sa; | |
| 431 | 449 | memset(&sa, 0, sizeof(sa)); | |
@@ -576,14 +594,18 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { | |||
| 576 | 594 | #else | |
| 577 | 595 | // Tell V8 to disable emitting WebAssembly | |
| 578 | 596 | // memory bounds checks. This means that we have | |
| 579 | - // to catch the SIGSEGV in TrapWebAssemblyOrContinue | ||
| 597 | + // to catch the SIGSEGV/SIGBUS in TrapWebAssemblyOrContinue | ||
| 580 | 598 | // and pass the signal context to V8. | |
| 581 | 599 | { | |
| 582 | 600 | struct sigaction sa; | |
| 583 | 601 | memset(&sa, 0, sizeof(sa)); | |
| 584 | 602 | sa.sa_sigaction = TrapWebAssemblyOrContinue; | |
| 585 | 603 | sa.sa_flags = SA_SIGINFO; | |
| 586 | 604 | 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 | ||
| 587 | 609 | } | |
| 588 | 610 | #endif // defined(_WIN32) | |
| 589 | 611 | V8::EnableWebAssemblyTrapHandler(false); | |
Binary file not shown.
| Original file line number | Diff line number | Diff 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 number | Diff line number | Diff 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