From ce76bbce3afb37cb9c713f9ad3e4bd50e5d38d11 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 18 Oct 2026 16:16:05 +0800 Subject: [PATCH 1/3] process: restore stdio blocking mode before execve Restore the startup O_NONBLOCK state of standard descriptors before execve(), preserving other file status flags and terminal settings. Save all flags before changing them so failed calls also restore shared stdout and stderr correctly. Assisted-by: Codex Signed-off-by: MK --- doc/api/process.md | 4 + src/node.cc | 23 +++++ src/node_internals.h | 1 + src/node_process_methods.cc | 85 ++++++++++------- test/fixtures/process-execve-stdio.py | 106 +++++++++++++++++++++ test/parallel/test-process-execve-stdio.js | 63 ++++++++++++ 6 files changed, 246 insertions(+), 36 deletions(-) create mode 100644 test/fixtures/process-execve-stdio.py create mode 100644 test/parallel/test-process-execve-stdio.js diff --git a/doc/api/process.md b/doc/api/process.md index 36e087b2cd0a..0437a5744a7b 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -1727,6 +1727,10 @@ This is achieved by using the `execve` POSIX function and therefore no memory or resources from the current process are preserved, except for the standard input, standard output and standard error file descriptor. +For standard input, standard output and standard error descriptors that still +refer to their original files or streams, the blocking mode returns to its state +at Node.js startup. Other file status flags and terminal settings remain unchanged. + On success, all other resources are discarded by the system when the processes are swapped, without triggering any exit or close events, without running any JavaScript cleanup handler (for example `process.on('exit')`), diff --git a/src/node.cc b/src/node.cc index e2efaad8a20c..0a35ea19e1bb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -667,6 +667,29 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { #endif // _WIN32 } +#ifdef __POSIX__ +int GetStdioFlagsForExec(int fd, int flags) { + CHECK_GE(fd, 0); + CHECK_LE(fd, STDERR_FILENO); + if (init_process_flags.load() & + ProcessInitializationFlags::kNoStdioInitialization) { + return flags; + } + + const auto& original = stdio[fd]; + struct stat current; + if (fstat(fd, ¤t) == -1) return -1; + + // Do not apply the startup state to a descriptor the program replaced. + if (original.stat.st_dev != current.st_dev || + original.stat.st_ino != current.st_ino) { + return flags; + } + + return (flags & ~O_NONBLOCK) | (original.flags & O_NONBLOCK); +} +#endif // __POSIX__ + // Safe to call more than once and from signal handlers. void ResetStdio() { if (init_process_flags.load() & diff --git a/src/node_internals.h b/src/node_internals.h index bb45d73e9090..5b751a6fdb5c 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -110,6 +110,7 @@ std::string FormatErrorMessage(v8::Isolate* isolate, bool add_source_line = true); void ResetStdio(); // Safe to call more than once and from signal handlers. #ifdef __POSIX__ +int GetStdioFlagsForExec(int fd, int flags); void SignalExit(int signal, siginfo_t* info, void* ucontext); #endif diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 6a35e5c5e4c4..f813ac64dc6a 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -511,22 +511,12 @@ static void ReallyExit(const FunctionCallbackInfo& args) { } #if defined __POSIX__ && !defined(__PASE__) -// Clears FD_CLOEXEC on `fd` so the descriptor is inherited across execve(2). -// On success returns the previous F_GETFD flags (>= 0) so callers can -// 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); - - if (flags < 0) { - return flags; - } - - if (fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC) < 0) { - return -1; - } - - return flags; +static int FcntlNoIntr(int fd, int command, int flags = 0) { + int result; + do { + result = fcntl(fd, command, flags); + } while (result == -1 && errno == EINTR); + return result; } static void Execve(const FunctionCallbackInfo& args) { @@ -579,22 +569,52 @@ 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}; + // Snapshot all descriptors before changing any flags: descriptors such as + // stdout and stderr may share an open file description (e.g. with 2>&1). + struct { + int descriptor_flags; + int status_flags; + int exec_flags; + } stdio_flags[3]; for (int fd = 0; fd < 3; fd++) { - int prev = persist_standard_stream(fd); - if (prev < 0) { - int fcntl_errno = errno; - // Undo changes already applied to earlier fds before throwing. - for (int j = 0; j < fd; j++) { - fcntl(j, F_SETFD, saved_stdio_flags[j]); + auto& flags = stdio_flags[fd]; + flags.descriptor_flags = FcntlNoIntr(fd, F_GETFD); + if (flags.descriptor_flags == -1) { + env->ThrowErrnoException(errno, "fcntl"); + return; + } + flags.status_flags = FcntlNoIntr(fd, F_GETFL); + if (flags.status_flags == -1) { + env->ThrowErrnoException(errno, "fcntl"); + return; + } + flags.exec_flags = GetStdioFlagsForExec(fd, flags.status_flags); + if (flags.exec_flags == -1) { + env->ThrowErrnoException(errno, "fstat"); + return; + } + } + + // A successful execve() never returns. On failure, restore the state needed + // by the current process before returning control to JavaScript. + auto restore_stdio = OnScopeLeave([&]() { + for (int fd = 0; fd < 3; fd++) { + const auto& flags = stdio_flags[fd]; + if (flags.exec_flags != flags.status_flags) { + FcntlNoIntr(fd, F_SETFL, flags.status_flags); } - env->ThrowErrnoException(fcntl_errno, "fcntl"); + FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); + } + }); + + for (int fd = 0; fd < 3; fd++) { + const auto& flags = stdio_flags[fd]; + if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == -1 || + (flags.exec_flags != flags.status_flags && + FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { + env->ThrowErrnoException(errno, "fcntl"); return; } - saved_stdio_flags[fd] = prev; } // Perform the execve operation. @@ -606,14 +626,7 @@ 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. - int execve_errno = errno; - for (int fd = 0; fd < 3; fd++) { - fcntl(fd, F_SETFD, saved_stdio_flags[fd]); - } - env->ThrowErrnoException(execve_errno, "execve", nullptr, *executable); + env->ThrowErrnoException(errno, "execve", nullptr, *executable); } #endif diff --git a/test/fixtures/process-execve-stdio.py b/test/fixtures/process-execve-stdio.py new file mode 100644 index 000000000000..7b6e0a750088 --- /dev/null +++ b/test/fixtures/process-execve-stdio.py @@ -0,0 +1,106 @@ +import contextlib +import fcntl +import os +import select +import socket +import subprocess +import sys +import tempfile + + +def status_flags(fd): + return fcntl.fcntl(fd, fcntl.F_GETFL) + + +def read_line(fd): + response = b'' + while not response.endswith(b'\n'): + ready, _, _ = select.select([fd], [], [], 10) + assert ready, 'Timed out waiting for child response' + chunk = os.read(fd, 4096) + assert chunk, response + response += chunk + return response + + +def run_case(node, script, kind, mask=0, mode='exec', alias=False): + with contextlib.ExitStack() as resources: + pairs = [] + for _ in range(3): + if kind == 'pipe': + pair = os.pipe() + else: + pair = tuple(endpoint.detach() for endpoint in socket.socketpair()) + for fd in pair: + resources.callback(os.close, fd) + pairs.append(pair) + + stdio = [pairs[0][0], pairs[1][1], pairs[2][1]] + if alias: + stdio[2] = stdio[1] + + for fd, stream in enumerate(stdio): + flags = status_flags(stream) + if mask & (1 << fd): + flags |= os.O_NONBLOCK + fcntl.fcntl(stream, fcntl.F_SETFL, flags) + + control_parent, control_child = (endpoint.detach() for endpoint in socket.socketpair()) + resources.callback(os.close, control_parent) + resources.callback(os.close, control_child) + args = [node, '--no-warnings', script, mode] + args += [sys.executable] if mode == 'exec' else [str(control_child)] + child = subprocess.Popen(args, stdin=stdio[0], stdout=stdio[1], + stderr=stdio[2], pass_fds=(control_child,)) + try: + if mode == 'exec': + response = read_line(pairs[1][0]) + expected = f'{mask} 0 0\n'.encode() + assert response == expected, (kind, mask, alias, response, expected) + else: + assert read_line(control_parent) == b'ready\n' + expected = [status_flags(fd) for fd in stdio] + assert all(flags & os.O_NONBLOCK for flags in expected), expected + os.write(control_parent, b'x') + assert read_line(control_parent) == b'ready\n' + actual = [status_flags(fd) for fd in stdio] + assert actual == expected, (kind, mode, alias, actual, expected) + os.write(control_parent, b'x') + assert child.wait(timeout=10) == 0, (kind, mask, mode, alias) + finally: + if child.poll() is None: + child.kill() + child.wait() + + +if sys.argv[1] == 'check': + nonblocking = 0 + close_on_exec = 0 + append = 0 + for fd in range(3): + if status_flags(fd) & os.O_NONBLOCK: + nonblocking |= 1 << fd + if fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC: + close_on_exec |= 1 << fd + if status_flags(fd) & os.O_APPEND: + append |= 1 << fd + os.write(1, f'{nonblocking} {close_on_exec} {append}\n'.encode()) +else: + node, script = sys.argv[2:] + for kind in ('pipe', 'socket'): + for mask in (0, 1, 2, 4, 7): + run_case(node, script, kind, mask) + for alias in (False, True): + if alias: + run_case(node, script, kind, mask=0, alias=True) + run_case(node, script, kind, mask=7, alias=True) + run_case(node, script, kind, mode='rollback', alias=alias) + run_case(node, script, kind, mode='closed', alias=alias) + + # A descriptor replaced after startup keeps its own file status flags. + with tempfile.NamedTemporaryFile() as output: + result = subprocess.run([node, '--no-warnings', script, 'reopened', + sys.executable, output.name], + input=b'', capture_output=True, timeout=10) + assert result.returncode == 0, result.stderr + assert output.read() == b'2 0 2\n' diff --git a/test/parallel/test-process-execve-stdio.js b/test/parallel/test-process-execve-stdio.js new file mode 100644 index 000000000000..75f939360a48 --- /dev/null +++ b/test/parallel/test-process-execve-stdio.js @@ -0,0 +1,63 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { closeSync, constants, openSync, readSync, writeSync } = require('fs'); +const { isMainThread } = require('worker_threads'); +const fixtures = require('../common/fixtures'); + +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'); +} + +const fixture = fixtures.path('process-execve-stdio.py'); +const mode = process.argv[2]; + +if (mode) { + // Opening the libuv handles makes the inherited pipes nonblocking. + assert.ok(process.stdin); + assert.ok(process.stdout); + assert.ok(process.stderr); + + if (mode === 'reopened') { + closeSync(1); + const flags = constants.O_WRONLY | constants.O_NONBLOCK | constants.O_APPEND; + assert.strictEqual(openSync(process.argv[4], flags), 1); + } + + if (mode === 'exec' || mode === 'reopened') { + const python = process.argv[3]; + process.execve(python, [python, fixture, 'check']); + assert.fail('process.execve unexpectedly returned'); + } + + // Keep the process alive while the parent checks the shared status flags. + // Normal process exit would restore them and conceal a failed rollback. + const control = Number(process.argv[3]); + function synchronize() { + writeSync(control, 'ready\n'); + assert.strictEqual(readSync(control, Buffer.alloc(1), 0, 1, null), 1); + } + + synchronize(); + if (mode === 'closed') closeSync(2); + assert.throws(() => { + process.execve(`${process.execPath}.nonexistent`, []); + }, { + code: mode === 'closed' ? 'EBADF' : 'ENOENT', + syscall: mode === 'closed' ? 'fcntl' : 'execve', + }); + + synchronize(); +} else { + const python = process.env.PYTHON || 'python3'; + const result = spawnSync(python, [fixture, 'run', process.execPath, __filename], { + encoding: 'utf8', + }); + assert.ifError(result.error); + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.signal, null); +} From e445d28bdf51622046a1d4203420140816eccd9f Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 18 Oct 2026 16:33:00 +0800 Subject: [PATCH 2/3] process: restore stdio before constructing execve errors Save errno and restore the descriptor and status flags before creating an error, whose inherited property setters can execute JavaScript. Check the flags from an inherited setter while the child remains alive. Keep the test cases and assertions in JavaScript and use Python for the POSIX descriptor setup and observations. Assisted-by: Codex Signed-off-by: MK --- src/node_process_methods.cc | 59 +++++++------ test/fixtures/process-execve-stdio.py | 82 +++++++++--------- test/parallel/test-process-execve-stdio.js | 99 +++++++++++++++++----- 3 files changed, 153 insertions(+), 87 deletions(-) diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index f813ac64dc6a..630daad80da2 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -595,38 +595,49 @@ static void Execve(const FunctionCallbackInfo& args) { } } - // A successful execve() never returns. On failure, restore the state needed - // by the current process before returning control to JavaScript. - auto restore_stdio = OnScopeLeave([&]() { + int err = 0; + const char* syscall = "fcntl"; + const char* path = nullptr; + { + // A successful execve() never returns. Restore stdio before constructing + // an error, which can invoke JavaScript setters on Error.prototype. + auto restore_stdio = OnScopeLeave([&]() { + for (int fd = 0; fd < 3; fd++) { + const auto& flags = stdio_flags[fd]; + if (flags.exec_flags != flags.status_flags) { + FcntlNoIntr(fd, F_SETFL, flags.status_flags); + } + FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); + } + }); + for (int fd = 0; fd < 3; fd++) { const auto& flags = stdio_flags[fd]; - if (flags.exec_flags != flags.status_flags) { - FcntlNoIntr(fd, F_SETFL, flags.status_flags); + if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == + -1 || + (flags.exec_flags != flags.status_flags && + FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { + err = errno; + break; } - FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); } - }); - for (int fd = 0; fd < 3; fd++) { - const auto& flags = stdio_flags[fd]; - if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == -1 || - (flags.exec_flags != flags.status_flags && - FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { - env->ThrowErrnoException(errno, "fcntl"); - return; + if (err == 0) { + // Perform the execve operation. + // + // Note: we intentionally do not invoke RunAtExit(env) here. On success + // the kernel discards the current address space when loading the new + // image, so any in-memory side effects of AtExit callbacks are lost + // anyway. On failure we want to leave the environment intact so the + // thrown exception can be observed and handled by JS code. + execve(*executable, argv.data(), envp.data()); + err = errno; + syscall = "execve"; + path = *executable; } } - // Perform the execve operation. - // - // Note: we intentionally do not invoke RunAtExit(env) here. On success the - // kernel discards the current address space when loading the new image, so - // any in-memory side effects of AtExit callbacks are lost anyway. On - // failure we want to leave the environment intact so the thrown exception - // can be observed and handled by JS code. - execve(*executable, argv.data(), envp.data()); - - env->ThrowErrnoException(errno, "execve", nullptr, *executable); + env->ThrowErrnoException(err, syscall, nullptr, path); } #endif diff --git a/test/fixtures/process-execve-stdio.py b/test/fixtures/process-execve-stdio.py index 7b6e0a750088..dd2d684ec36c 100644 --- a/test/fixtures/process-execve-stdio.py +++ b/test/fixtures/process-execve-stdio.py @@ -1,21 +1,29 @@ import contextlib import fcntl +import json import os import select import socket import subprocess import sys -import tempfile + +# Node.js has no fcntl() API, and its startup can change inherited stdio flags. +# Use Python to prepare and inspect descriptors without those changes. The +# JavaScript test owns the test cases and checks the observations returned here. def status_flags(fd): return fcntl.fcntl(fd, fcntl.F_GETFL) -def read_line(fd): +def nonblocking_mask(flags): + return sum(1 << fd for fd, flag in enumerate(flags) if flag & os.O_NONBLOCK) + + +def read_line(fd, timeout): response = b'' while not response.endswith(b'\n'): - ready, _, _ = select.select([fd], [], [], 10) + ready, _, _ = select.select([fd], [], [], timeout) assert ready, 'Timed out waiting for child response' chunk = os.read(fd, 4096) assert chunk, response @@ -23,7 +31,7 @@ def read_line(fd): return response -def run_case(node, script, kind, mask=0, mode='exec', alias=False): +def run_case(node, script, kind, mode, mask, alias, timeout): with contextlib.ExitStack() as resources: pairs = [] for _ in range(3): @@ -45,28 +53,30 @@ def run_case(node, script, kind, mask=0, mode='exec', alias=False): flags |= os.O_NONBLOCK fcntl.fcntl(stream, fcntl.F_SETFL, flags) - control_parent, control_child = (endpoint.detach() for endpoint in socket.socketpair()) - resources.callback(os.close, control_parent) - resources.callback(os.close, control_child) + control_parent, control_child = socket.socketpair() + resources.enter_context(control_parent) + resources.enter_context(control_child) args = [node, '--no-warnings', script, mode] - args += [sys.executable] if mode == 'exec' else [str(control_child)] + args += [sys.executable] if mode == 'exec' else [str(control_child.fileno())] child = subprocess.Popen(args, stdin=stdio[0], stdout=stdio[1], - stderr=stdio[2], pass_fds=(control_child,)) + stderr=stdio[2], pass_fds=(control_child.fileno(),)) + control_child.close() try: if mode == 'exec': - response = read_line(pairs[1][0]) - expected = f'{mask} 0 0\n'.encode() - assert response == expected, (kind, mask, alias, response, expected) + result = {'flags': json.loads(read_line(pairs[1][0], timeout))} else: - assert read_line(control_parent) == b'ready\n' - expected = [status_flags(fd) for fd in stdio] - assert all(flags & os.O_NONBLOCK for flags in expected), expected - os.write(control_parent, b'x') - assert read_line(control_parent) == b'ready\n' - actual = [status_flags(fd) for fd in stdio] - assert actual == expected, (kind, mode, alias, actual, expected) - os.write(control_parent, b'x') - assert child.wait(timeout=10) == 0, (kind, mask, mode, alias) + snapshots = [] + while True: + phase = read_line(control_parent.fileno(), timeout).decode().strip() + flags = [status_flags(fd) for fd in stdio] + snapshots.append({'phase': phase, 'flags': flags, + 'nonblocking': nonblocking_mask(flags)}) + control_parent.sendall(b'x') + if phase == 'after': + break + result = {'snapshots': snapshots} + result['status'] = child.wait(timeout=timeout) + return result finally: if child.poll() is None: child.kill() @@ -74,33 +84,17 @@ def run_case(node, script, kind, mask=0, mode='exec', alias=False): if sys.argv[1] == 'check': - nonblocking = 0 + flags = [status_flags(fd) for fd in range(3)] close_on_exec = 0 append = 0 for fd in range(3): - if status_flags(fd) & os.O_NONBLOCK: - nonblocking |= 1 << fd if fcntl.fcntl(fd, fcntl.F_GETFD) & fcntl.FD_CLOEXEC: close_on_exec |= 1 << fd - if status_flags(fd) & os.O_APPEND: + if flags[fd] & os.O_APPEND: append |= 1 << fd - os.write(1, f'{nonblocking} {close_on_exec} {append}\n'.encode()) + result = {'nonblocking': nonblocking_mask(flags), + 'closeOnExec': close_on_exec, 'append': append} + os.write(1, (json.dumps(result) + '\n').encode()) else: - node, script = sys.argv[2:] - for kind in ('pipe', 'socket'): - for mask in (0, 1, 2, 4, 7): - run_case(node, script, kind, mask) - for alias in (False, True): - if alias: - run_case(node, script, kind, mask=0, alias=True) - run_case(node, script, kind, mask=7, alias=True) - run_case(node, script, kind, mode='rollback', alias=alias) - run_case(node, script, kind, mode='closed', alias=alias) - - # A descriptor replaced after startup keeps its own file status flags. - with tempfile.NamedTemporaryFile() as output: - result = subprocess.run([node, '--no-warnings', script, 'reopened', - sys.executable, output.name], - input=b'', capture_output=True, timeout=10) - assert result.returncode == 0, result.stderr - assert output.read() == b'2 0 2\n' + node, script, options = sys.argv[2:] + print(json.dumps(run_case(node, script, **json.loads(options)))) diff --git a/test/parallel/test-process-execve-stdio.js b/test/parallel/test-process-execve-stdio.js index 75f939360a48..74ca6972c16d 100644 --- a/test/parallel/test-process-execve-stdio.js +++ b/test/parallel/test-process-execve-stdio.js @@ -1,11 +1,12 @@ 'use strict'; const common = require('../common'); -const assert = require('assert'); -const { spawnSync } = require('child_process'); -const { closeSync, constants, openSync, readSync, writeSync } = require('fs'); -const { isMainThread } = require('worker_threads'); +const assert = require('node:assert'); +const { closeSync, constants, openSync, readFileSync, readSync, writeFileSync, writeSync } = require('node:fs'); +const { isMainThread } = require('node:worker_threads'); +const { spawnSyncAndAssert } = require('../common/child_process'); const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); if (!isMainThread) { common.skip('process.execve is not available in Workers'); @@ -37,27 +38,87 @@ if (mode) { // Keep the process alive while the parent checks the shared status flags. // Normal process exit would restore them and conceal a failed rollback. const control = Number(process.argv[3]); - function synchronize() { - writeSync(control, 'ready\n'); + function synchronize(phase) { + writeSync(control, `${phase}\n`); assert.strictEqual(readSync(control, Buffer.alloc(1), 0, 1, null), 1); } - synchronize(); + synchronize('before'); if (mode === 'closed') closeSync(2); - assert.throws(() => { - process.execve(`${process.execPath}.nonexistent`, []); - }, { - code: mode === 'closed' ? 'EBADF' : 'ENOENT', - syscall: mode === 'closed' ? 'fcntl' : 'execve', + Object.defineProperty(Error.prototype, 'code', { + configurable: true, + get: common.mustNotCall(), + set: common.mustCall(function(code) { + // Error construction can execute JavaScript before execve() throws. + // The descriptor flags must already be restored at this point. + synchronize('setter'); + Object.defineProperty(this, 'code', { + value: code, + writable: true, + enumerable: true, + configurable: true, + }); + }), }); + try { + assert.throws(() => { + process.execve(`${process.execPath}.nonexistent`, []); + }, { + code: mode === 'closed' ? 'EBADF' : 'ENOENT', + syscall: mode === 'closed' ? 'fcntl' : 'execve', + }); + } finally { + delete Error.prototype.code; + } - synchronize(); + synchronize('after'); } else { - const python = process.env.PYTHON || 'python3'; - const result = spawnSync(python, [fixture, 'run', process.execPath, __filename], { - encoding: 'utf8', + // execve() needs the interpreter path because it does not search PATH. + const { child } = spawnSyncAndAssert(process.env.PYTHON || 'python3', [ + '-c', 'import sys; print(sys.executable)', + ], {}); + const python = child.stdout.toString().trim(); + const timeout = common.platformTimeout(10_000) / 1000; + + function runCase(kind, mode = 'exec', mask = 0, alias = false) { + const options = JSON.stringify({ kind, mode, mask, alias, timeout }); + spawnSyncAndAssert(python, [fixture, 'run', process.execPath, __filename, options], { + stdout(output) { + const { status, flags, snapshots } = JSON.parse(output); + assert.strictEqual(status, 0); + if (mode === 'exec') { + assert.deepStrictEqual(flags, { nonblocking: mask, closeOnExec: 0, append: 0 }); + } else { + assert.deepStrictEqual(snapshots.map(({ phase }) => phase), ['before', 'setter', 'after']); + assert.strictEqual(snapshots[0].nonblocking, 7); + for (const snapshot of snapshots.slice(1)) { + assert.deepStrictEqual(snapshot.flags, snapshots[0].flags, snapshot.phase); + } + } + }, + }); + } + + for (const kind of ['pipe', 'socket']) { + for (const mask of [0, 1, 2, 4, 7]) { + runCase(kind, 'exec', mask); + } + for (const alias of [false, true]) { + if (alias) { + runCase(kind, 'exec', 0, alias); + runCase(kind, 'exec', 7, alias); + } + runCase(kind, 'rollback', 0, alias); + runCase(kind, 'closed', 0, alias); + } + } + + // A descriptor replaced after startup keeps its own file status flags. + tmpdir.refresh(); + const output = tmpdir.resolve('execve-stdio'); + writeFileSync(output, ''); + spawnSyncAndAssert(process.execPath, ['--no-warnings', __filename, 'reopened', python, output], {}); + assert.deepStrictEqual(JSON.parse(readFileSync(output, 'utf8')), { + nonblocking: 2, closeOnExec: 0, append: 2, }); - assert.ifError(result.error); - assert.strictEqual(result.status, 0, result.stderr); - assert.strictEqual(result.signal, null); } From 7f9bfc7a9f94f26574a356d284b902f8b79b8ad4 Mon Sep 17 00:00:00 2001 From: MK Date: Sun, 18 Oct 2026 17:41:28 +0800 Subject: [PATCH 3/3] process: simplify execve stdio rollback Restore stdio explicitly at both failure exits after saving errno and before constructing the JavaScript error. Remove the cleanup scope and error-routing variables while preserving the blocking-mode policy. Clarify that the startup-mode check compares the underlying file. Assisted-by: Codex Signed-off-by: MK --- src/node.cc | 2 +- src/node_process_methods.cc | 56 ++++++++-------------- test/parallel/test-process-execve-stdio.js | 2 +- 3 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/node.cc b/src/node.cc index 0a35ea19e1bb..6a8a3ef54d70 100644 --- a/src/node.cc +++ b/src/node.cc @@ -680,7 +680,7 @@ int GetStdioFlagsForExec(int fd, int flags) { struct stat current; if (fstat(fd, ¤t) == -1) return -1; - // Do not apply the startup state to a descriptor the program replaced. + // Only apply the startup mode while the descriptor refers to the same file. if (original.stat.st_dev != current.st_dev || original.stat.st_ino != current.st_ino) { return flags; diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 630daad80da2..bc7d33ef7cfa 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -595,49 +595,35 @@ static void Execve(const FunctionCallbackInfo& args) { } } - int err = 0; - const char* syscall = "fcntl"; - const char* path = nullptr; - { - // A successful execve() never returns. Restore stdio before constructing - // an error, which can invoke JavaScript setters on Error.prototype. - auto restore_stdio = OnScopeLeave([&]() { - for (int fd = 0; fd < 3; fd++) { - const auto& flags = stdio_flags[fd]; - if (flags.exec_flags != flags.status_flags) { - FcntlNoIntr(fd, F_SETFL, flags.status_flags); - } - FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); - } - }); - + auto restore_stdio = [&]() { for (int fd = 0; fd < 3; fd++) { const auto& flags = stdio_flags[fd]; - if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == - -1 || - (flags.exec_flags != flags.status_flags && - FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { - err = errno; - break; + if (flags.exec_flags != flags.status_flags) { + FcntlNoIntr(fd, F_SETFL, flags.status_flags); } + FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags); } + }; - if (err == 0) { - // Perform the execve operation. - // - // Note: we intentionally do not invoke RunAtExit(env) here. On success - // the kernel discards the current address space when loading the new - // image, so any in-memory side effects of AtExit callbacks are lost - // anyway. On failure we want to leave the environment intact so the - // thrown exception can be observed and handled by JS code. - execve(*executable, argv.data(), envp.data()); - err = errno; - syscall = "execve"; - path = *executable; + for (int fd = 0; fd < 3; fd++) { + const auto& flags = stdio_flags[fd]; + if (FcntlNoIntr(fd, F_SETFD, flags.descriptor_flags & ~FD_CLOEXEC) == -1 || + (flags.exec_flags != flags.status_flags && + FcntlNoIntr(fd, F_SETFL, flags.exec_flags) == -1)) { + const int err = errno; + restore_stdio(); + env->ThrowErrnoException(err, "fcntl"); + return; } } - env->ThrowErrnoException(err, syscall, nullptr, path); + // Do not run AtExit callbacks: a failed execve() must leave Node.js usable. + execve(*executable, argv.data(), envp.data()); + + // Error construction can invoke JavaScript setters. Restore stdio first. + const int err = errno; + restore_stdio(); + env->ThrowErrnoException(err, "execve", nullptr, *executable); } #endif diff --git a/test/parallel/test-process-execve-stdio.js b/test/parallel/test-process-execve-stdio.js index 74ca6972c16d..85442e7a8f66 100644 --- a/test/parallel/test-process-execve-stdio.js +++ b/test/parallel/test-process-execve-stdio.js @@ -113,7 +113,7 @@ if (mode) { } } - // A descriptor replaced after startup keeps its own file status flags. + // A descriptor redirected to a different file keeps its own status flags. tmpdir.refresh(); const output = tmpdir.resolve('execve-stdio'); writeFileSync(output, '');