From 08d0f50793bafb13414bd13dd8d27fa1f8687b84 Mon Sep 17 00:00:00 2001 From: AdamMagued Date: Sat, 24 Oct 2026 21:00:17 +0300 Subject: [PATCH] process: restore blocking stdio before execve When Node.js initializes stdio handles (such as process.stdout or process.stderr), libuv configures O_NONBLOCK on the open file descriptions for fds 0, 1, and 2. Because process.execve() replaces the process image without invoking the standard exit cleanup path (which triggers ResetStdio()), the executed binary inherits fds 0-2 in non-blocking mode. Programs assuming blocking stdio fail with EAGAIN or BlockingIOError when writes exceed pipe capacity. Clear O_NONBLOCK on standard descriptors 0, 1, and 2 before invoking execve(2), matching the behavior of child_process.spawn(). If execve(2) subsequently fails, restore the previous file status flags alongside the file descriptor close-on-exec flags so no side effects remain. Fixes: https://github.com/nodejs/node/issues/66824 Signed-off-by: AdamMagued --- src/node_process_methods.cc | 101 +++++++++++++-- .../test-process-execve-stdio-blocking.js | 120 ++++++++++++++++++ 2 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-process-execve-stdio-blocking.js diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 6a35e5c5e4c4..22527586cf82 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -516,19 +516,73 @@ static void ReallyExit(const FunctionCallbackInfo& args) { // restore them if execve(2) subsequently fails. On failure returns -1 with // errno set. inline int persist_standard_stream(int fd) { - int flags = fcntl(fd, F_GETFD, 0); + int flags; + do { + flags = fcntl(fd, F_GETFD, 0); + } while (flags < 0 && errno == EINTR); if (flags < 0) { return flags; } - if (fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC) < 0) { + int err; + do { + err = fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC); + } while (err < 0 && errno == EINTR); + + if (err < 0) { return -1; } return flags; } +// Clears O_NONBLOCK on `fd` so the descriptor is blocking in the replacement +// process, matching child_process.spawn() and normal exit behavior. +// On success returns the previous F_GETFL flags (>= 0) so callers can +// restore them if execve(2) subsequently fails. On failure returns -1 with +// errno set. +inline int restore_standard_stream_blocking(int fd) { + int flags; + do { + flags = fcntl(fd, F_GETFL, 0); + } while (flags < 0 && errno == EINTR); + + if (flags < 0) { + return flags; + } + + if (flags & O_NONBLOCK) { + int err; + do { + err = fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); + } while (err < 0 && errno == EINTR); + + if (err < 0) { + return -1; + } + } + + return flags; +} + +inline void restore_standard_stream_flags(int fd, + int saved_fd_flags, + int saved_fl_flags) { + if (saved_fd_flags >= 0) { + int err; + do { + err = fcntl(fd, F_SETFD, saved_fd_flags); + } while (err < 0 && errno == EINTR); + } + if (saved_fl_flags >= 0 && (saved_fl_flags & O_NONBLOCK)) { + int err; + do { + err = fcntl(fd, F_SETFL, saved_fl_flags); + } while (err < 0 && errno == EINTR); + } +} + static void Execve(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -580,21 +634,40 @@ static void Execve(const FunctionCallbackInfo& args) { envp[envp_array->Length()] = nullptr; // Set stdin, stdout and stderr to be non-close-on-exec so that the new - // process will inherit them. Save the previous flags on each fd so we can - // restore them if execve(2) fails and we throw back to JS. - int saved_stdio_flags[3] = {-1, -1, -1}; + // process will inherit them, and restore blocking mode so that the new + // process does not inherit O_NONBLOCK set by libuv. Save the previous + // flags on each fd so we can restore them if execve(2) fails and we throw + // back to JS. + int saved_stdio_fd_flags[3] = {-1, -1, -1}; + int saved_stdio_fl_flags[3] = {-1, -1, -1}; for (int fd = 0; fd < 3; fd++) { - int prev = persist_standard_stream(fd); - if (prev < 0) { + int prev_fd = persist_standard_stream(fd); + if (prev_fd < 0) { + int fcntl_errno = errno; + // Undo changes already applied to earlier fds before throwing. + for (int j = 0; j < fd; j++) { + restore_standard_stream_flags( + j, saved_stdio_fd_flags[j], saved_stdio_fl_flags[j]); + } + env->ThrowErrnoException(fcntl_errno, "fcntl"); + return; + } + saved_stdio_fd_flags[fd] = prev_fd; + + int prev_fl = restore_standard_stream_blocking(fd); + if (prev_fl < 0) { int fcntl_errno = errno; + // Undo F_SETFD on current fd. + restore_standard_stream_flags(fd, saved_stdio_fd_flags[fd], -1); // Undo changes already applied to earlier fds before throwing. for (int j = 0; j < fd; j++) { - fcntl(j, F_SETFD, saved_stdio_flags[j]); + restore_standard_stream_flags( + j, saved_stdio_fd_flags[j], saved_stdio_fl_flags[j]); } env->ThrowErrnoException(fcntl_errno, "fcntl"); return; } - saved_stdio_flags[fd] = prev; + saved_stdio_fl_flags[fd] = prev_fl; } // Perform the execve operation. @@ -606,12 +679,13 @@ static void Execve(const FunctionCallbackInfo& args) { // can be observed and handled by JS code. execve(*executable, argv.data(), envp.data()); - // If execve returned, it failed. Restore the FD_CLOEXEC flags we cleared - // above so that a failed call leaves no observable side effects, then - // throw an ErrnoException so JS can catch it. + // If execve returned, it failed. Restore the FD_CLOEXEC and O_NONBLOCK flags + // we modified above so that a failed call leaves no observable side effects, + // then throw an ErrnoException so JS can catch it. int execve_errno = errno; for (int fd = 0; fd < 3; fd++) { - fcntl(fd, F_SETFD, saved_stdio_flags[fd]); + restore_standard_stream_flags( + fd, saved_stdio_fd_flags[fd], saved_stdio_fl_flags[fd]); } env->ThrowErrnoException(execve_errno, "execve", nullptr, *executable); } @@ -878,3 +952,4 @@ NODE_BINDING_PER_ISOLATE_INIT(process_methods, node::process::CreatePerIsolateProperties) NODE_BINDING_EXTERNAL_REFERENCE(process_methods, node::process::RegisterExternalReferences) + diff --git a/test/parallel/test-process-execve-stdio-blocking.js b/test/parallel/test-process-execve-stdio-blocking.js new file mode 100644 index 000000000000..b549db32f164 --- /dev/null +++ b/test/parallel/test-process-execve-stdio-blocking.js @@ -0,0 +1,120 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { execFileSync, spawn, spawnSync } = require('child_process'); +const { isMainThread } = require('worker_threads'); + +if (!isMainThread) { + common.skip('process.execve is not available in Workers'); +} else if (common.isWindows || common.isIBMi) { + common.skip('process.execve is not available in Windows or IBM i'); +} + +let pythonPath; +try { + pythonPath = execFileSync('which', ['python3'], { encoding: 'utf8' }).trim(); +} catch { + if (process.env.PYTHON) { + pythonPath = process.env.PYTHON; + } +} + +if (!pythonPath) { + common.skip('python3 is required for this test'); +} + +if (process.argv[2] === 'check-flags') { + // Touch stdio handles to initialize libuv and set O_NONBLOCK. + assert.ok(process.stdin); + assert.ok(process.stdout); + assert.ok(process.stderr); + + const pythonScript = [ + 'import fcntl, os, sys', + 'for fd in (0, 1, 2):', + ' try:', + ' flags = fcntl.fcntl(fd, fcntl.F_GETFL)', + ' except OSError:', + ' continue', + ' if flags & os.O_NONBLOCK:', + ' sys.stderr.write(f"fd {fd} has O_NONBLOCK\\n")', + ' sys.exit(1)', + 'print("SUCCESS_BLOCKING")', + ].join('\n'); + + process.execve(pythonPath, ['python3', '-c', pythonScript], process.env); + assert.fail('process.execve should not return on success'); +} else if (process.argv[2] === 'check-pipe-buffer') { + assert.ok(process.stdout); + + const pythonScript = [ + 'import sys', + 'data = b"x" * 262144', + 'sys.stdout.buffer.write(data)', + 'sys.stdout.buffer.flush()', + ].join('\n'); + + process.execve(pythonPath, ['python3', '-c', pythonScript], process.env); + assert.fail('process.execve should not return on success'); +} + +// 1. Verify that fcntl F_GETFL does not contain O_NONBLOCK after process.execve() +{ + const result = spawnSync(process.execPath, [__filename, 'check-flags'], { + stdio: ['pipe', 'pipe', 'pipe'], + encoding: 'utf8', + }); + + assert.strictEqual( + result.status, + 0, + `Child exited with code ${result.status}, stderr: ${result.stderr}` + ); + assert.match(result.stdout, /SUCCESS_BLOCKING/); +} + +// 2. Verify that writing more than pipe buffer capacity succeeds without EAGAIN +{ + const child = spawn(process.execPath, [__filename, 'check-pipe-buffer'], { + stdio: ['inherit', 'pipe', 'pipe'], + }); + + let receivedBytes = 0; + child.stdout.on('data', (chunk) => { + receivedBytes += chunk.length; + }); + + let stderr = ''; + child.stderr.on('data', (chunk) => { + stderr += chunk; + }); + + child.on('close', common.mustCall((code) => { + assert.strictEqual(code, 0, `Child exited with code ${code}, stderr: ${stderr}`); + assert.strictEqual(receivedBytes, 262144); + })); +} + +// 3. Verify that if process.execve() fails, stdio streams remain writable +{ + assert.ok(process.stdout); + assert.ok(process.stderr); + + assert.throws( + () => { + process.execve( + `${process.execPath}_non_existing_executable`, + [process.execPath, 'arg'], + process.env + ); + }, + { + code: 'ENOENT', + syscall: 'execve', + } + ); + + assert.strictEqual(process.stdout.writable, true); + assert.strictEqual(process.stderr.writable, true); +}