Skip to content

Commit

Permalink
src: reset SIGSEGV handler before crashing
Browse files Browse the repository at this point in the history
Without this, we would re-enter the signal handler immediately
after re-raising the signal, leading to an infinite loop.

PR-URL: #27775
Refs: #27246
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
addaleax committed Jun 14, 2019
1 parent e6b3ec3 commit e256204
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,12 @@ void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) {
if (prev != nullptr) {
prev(signo, info, ucontext);
} else {
// Reset to the default signal handler, i.e. cause a hard crash.
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = SIG_DFL;
CHECK_EQ(sigaction(signo, &sa, nullptr), 0);

ResetStdio();
raise(signo);
}
Expand Down
7 changes: 7 additions & 0 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ static void Abort(const FunctionCallbackInfo<Value>& args) {
Abort();
}

// For internal testing only, not exposed to userland.
static void CauseSegfault(const FunctionCallbackInfo<Value>& args) {
// This should crash hard all platforms.
*static_cast<void**>(nullptr) = nullptr;
}

static void Chdir(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK(env->owns_process_state());
Expand Down Expand Up @@ -405,6 +411,7 @@ static void InitializeProcessMethods(Local<Object> target,
env->SetMethod(target, "_debugProcess", DebugProcess);
env->SetMethod(target, "_debugEnd", DebugEnd);
env->SetMethod(target, "abort", Abort);
env->SetMethod(target, "causeSegfault", CauseSegfault);
env->SetMethod(target, "chdir", Chdir);
}

Expand Down
23 changes: 23 additions & 0 deletions test/abort/test-signal-handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
if (common.isWindows)
common.skip('No signals on Window');

const assert = require('assert');
const { spawnSync } = require('child_process');

// Test that a hard crash does not cause an endless loop.

if (process.argv[2] === 'child') {
const { internalBinding } = require('internal/test/binding');
const { causeSegfault } = internalBinding('process_methods');

causeSegfault();
} else {
const child = spawnSync(process.execPath,
['--expose-internals', __filename, 'child'],
{ stdio: 'inherit' });
// FreeBSD and macOS use SIGILL for the kind of crash we're causing here.
assert(child.signal === 'SIGSEGV' || child.signal === 'SIGILL',
`child.signal = ${child.signal}`);
}

0 comments on commit e256204

Please sign in to comment.