Skip to content

Commit

Permalink
src: fix spawnSync CHECK when SIGKILL fails
Browse files Browse the repository at this point in the history
We might not have sufficient privileges to signal the child process
so don't make assumptions about the return value of `uv_process_kill()`.

Example:

    node -e 'child_process.spawnSync("sudo", ["ls"], { maxBuffer: 1 })'

No test because:

1. The test needs to run as root (can't invoke sudo), and

2. The parent needs to drop privileges but can't, because
   then the child process won't have sufficient privileges.

Fixes: nodejs#31747
  • Loading branch information
bnoordhuis committed Feb 13, 2020
1 parent a751389 commit 7bd387a
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/spawn_sync.cc
Expand Up @@ -607,8 +607,9 @@ void SyncProcessRunner::Kill() {
if (r < 0 && r != UV_ESRCH) {
SetError(r);

r = uv_process_kill(&uv_process_, SIGKILL);
CHECK(r >= 0 || r == UV_ESRCH);
// Deliberately ignore the return value, we might not have
// sufficient privileges to signal the child process.
USE(uv_process_kill(&uv_process_, SIGKILL));
}
}

Expand Down

0 comments on commit 7bd387a

Please sign in to comment.