From f223d2c7e4fdd5271d38230a04cb4cbafc71101d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 13 Feb 2020 12:40:53 +0100 Subject: [PATCH] src: fix spawnSync CHECK when SIGKILL fails 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: https://github.com/nodejs/node/issues/31747 PR-URL: https://github.com/nodejs/node/pull/31768 Reviewed-By: Santiago Gimeno Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: David Carlier --- src/spawn_sync.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 589b77f6c1eb95..78e474cd8253e3 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -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)); } }