From 90715fbefa6d208ebebec77a6ef8349237deadb6 Mon Sep 17 00:00:00 2001 From: Caner Altinbasak Date: Thu, 25 Jun 2026 12:01:23 +0100 Subject: [PATCH 1/2] fix: skip native write for zero-length callback writeFile OS-20458 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v8 built with V8_ENABLE_SANDBOX in Chromium. Every Buffer's bytes must live inside the V8 memory cage, and pointers are cage-relative. A zero-length buffer has no allocation in the cage, so Buffer::Data() resolves to the cage base / an empty-store sentinel — an address the kernel rejects when it's handed to write(2), even with len == 0 Plain Node, V8 sandbox off. Buffers come from Node's normal heap allocator; an empty buffer yields an ordinary valid pointer, so write(fd, ptr, 0) is a harmless no-op → returns 0. Callback fs.writeFile()/writeAll() always issued a native fs.write(fd, buffer, 0, 0, ...) syscall even when the data was empty. On the Electron-based roHtmlWidget Node integration that zero-length write returns "EFAULT: bad address in system call argument, write", whereas it succeeds under the standalone roNodeJs runtime and under fs/promises.writeFile(). Add a zero-length guard to writeAll() that skips the syscall and runs the normal completion path (optional fsync, close the fd only when writeFile() opened it, invoke callback(null)), matching the early return already present in fs/promises.writeFile(). Non-empty writes, validation, abort/signal handling and caller-owned fd semantics are unchanged. --- patches/node/.patches | 1 + ...e_for_zero-length_callback_writefile.patch | 96 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 patches/node/fs_skip_native_write_for_zero-length_callback_writefile.patch diff --git a/patches/node/.patches b/patches/node/.patches index be1399867bac1..e57ab300b639c 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -45,3 +45,4 @@ src_refactor_wasmstreaming_finish_to_accept_a_callback.patch src_stop_using_v8_propertycallbackinfo_t_this.patch build_restore_macos_deployment_target_to_12_0.patch fix_add_externalpointertypetag_to_v8_external_api_calls.patch +fs_skip_native_write_for_zero-length_callback_writefile.patch diff --git a/patches/node/fs_skip_native_write_for_zero-length_callback_writefile.patch b/patches/node/fs_skip_native_write_for_zero-length_callback_writefile.patch new file mode 100644 index 0000000000000..d8e06e062880d --- /dev/null +++ b/patches/node/fs_skip_native_write_for_zero-length_callback_writefile.patch @@ -0,0 +1,96 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Caner Altinbasak +Date: Thu, 25 Jun 2026 10:51:20 +0100 +Subject: fs: skip native write for zero-length callback writeFile() + +Callback fs.writeFile()/writeAll() always issued a native +fs.write(fd, buffer, 0, 0, ...) syscall even when the data was empty. +On the Electron-based roHtmlWidget Node integration that zero-length +write returns "EFAULT: bad address in system call argument, write", +whereas it succeeds under the standalone roNodeJs runtime and under +fs/promises.writeFile(). + +Add a zero-length guard to writeAll() that skips the syscall and runs +the normal completion path (optional fsync, close the fd only when +writeFile() opened it, invoke callback(null)), matching the early +return already present in fs/promises.writeFile(). Non-empty writes, +validation, abort/signal handling and caller-owned fd semantics are +unchanged. + +diff --git a/lib/fs.js b/lib/fs.js +index 6e688375f48d2d6a12e0af6ac0c94c84b9c3c9f4..05c0b69e68cd26a862153dd3cf10cc94d7c0da32 100644 +--- a/lib/fs.js ++++ b/lib/fs.js +@@ -2283,6 +2283,41 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) + } + return; + } ++ // Shared completion path: optionally flush, close the fd if writeFile() ++ // opened it internally, and invoke the callback. ++ const onWriteSucceeded = () => { ++ if (!flush) { ++ if (isUserFd) { ++ callback(null); ++ } else { ++ fs.close(fd, callback); ++ } ++ } else { ++ fs.fsync(fd, (syncErr) => { ++ if (syncErr) { ++ if (isUserFd) { ++ callback(syncErr); ++ } else { ++ fs.close(fd, (err) => { ++ callback(aggregateTwoErrors(err, syncErr)); ++ }); ++ } ++ } else if (isUserFd) { ++ callback(null); ++ } else { ++ fs.close(fd, callback); ++ } ++ }); ++ } ++ }; ++ // A zero-length write would still issue a native fs.write(fd, buffer, 0, 0, ++ // ...) syscall, which returns EFAULT on some runtimes (notably the ++ // Electron-based roHtmlWidget Node integration). Skip the syscall and finish ++ // as a successful 0-byte write, matching fs/promises.writeFile(). ++ if (length === 0) { ++ onWriteSucceeded(); ++ return; ++ } + // write(fd, buffer, offset, length, position, callback) + fs.write(fd, buffer, offset, length, null, (writeErr, written) => { + if (writeErr) { +@@ -2294,29 +2329,7 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) + }); + } + } else if (written === length) { +- if (!flush) { +- if (isUserFd) { +- callback(null); +- } else { +- fs.close(fd, callback); +- } +- } else { +- fs.fsync(fd, (syncErr) => { +- if (syncErr) { +- if (isUserFd) { +- callback(syncErr); +- } else { +- fs.close(fd, (err) => { +- callback(aggregateTwoErrors(err, syncErr)); +- }); +- } +- } else if (isUserFd) { +- callback(null); +- } else { +- fs.close(fd, callback); +- } +- }); +- } ++ onWriteSucceeded(); + } else { + offset += written; + length -= written; From 3ba69c73068936f17dc6d182ab037c620ec085ee Mon Sep 17 00:00:00 2001 From: Caner Altinbasak Date: Thu, 25 Jun 2026 15:44:19 +0100 Subject: [PATCH 2/2] test: zero-length callback fs.writeFile in renderer OS-20458 Add renderer (nodeIntegration / roHtmlWidget) regression tests for the zero-length callback fs.writeFile() EFAULT fix. Covers empty string and empty Buffer writes, a non-empty write, truncation of an existing file, and a caller-owned fd that must not be closed by fs.writeFile(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- spec/node-spec.ts | 109 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/spec/node-spec.ts b/spec/node-spec.ts index 7660fbe46c08b..652943e78de2e 100644 --- a/spec/node-spec.ts +++ b/spec/node-spec.ts @@ -400,6 +400,115 @@ describe('node feature', () => { ); }); + // Regression test for a zero-length callback fs.writeFile() that failed + // with "EFAULT: bad address in system call argument, write" only in the + // renderer (nodeIntegration / roHtmlWidget) Node integration, while it + // worked in a standalone Node process and via fs/promises.writeFile(). + // Callback fs.writeFile() now skips the native zero-length write, matching + // the early return already present in fs/promises.writeFile(). + describe('zero-length fs.writeFile in renderer', () => { + itremote('fs.writeFile(path, "", cb) succeeds and creates an empty file', async () => { + const fs = require('node:fs'); + const path = require('node:path'); + const file = path.join(require('node:os').tmpdir(), `electron-fswrite-empty-string-${Date.now()}`); + try { + await new Promise((resolve, reject) => { + fs.writeFile(file, '', (err: Error | null) => (err ? reject(err) : resolve())); + }); + expect(fs.statSync(file).size).to.equal(0); + } finally { + try { + fs.unlinkSync(file); + } catch { + /* ignore */ + } + } + }); + + itremote('fs.writeFile(path, Buffer.alloc(0), cb) succeeds', async () => { + const fs = require('node:fs'); + const path = require('node:path'); + const file = path.join(require('node:os').tmpdir(), `electron-fswrite-empty-buffer-${Date.now()}`); + try { + await new Promise((resolve, reject) => { + fs.writeFile(file, Buffer.alloc(0), (err: Error | null) => (err ? reject(err) : resolve())); + }); + expect(fs.statSync(file).size).to.equal(0); + } finally { + try { + fs.unlinkSync(file); + } catch { + /* ignore */ + } + } + }); + + itremote('fs.writeFile(path, "1", cb) still writes non-empty data', async () => { + const fs = require('node:fs'); + const path = require('node:path'); + const file = path.join(require('node:os').tmpdir(), `electron-fswrite-non-empty-${Date.now()}`); + try { + await new Promise((resolve, reject) => { + fs.writeFile(file, '1', (err: Error | null) => (err ? reject(err) : resolve())); + }); + expect(fs.readFileSync(file, 'utf-8')).to.equal('1'); + } finally { + try { + fs.unlinkSync(file); + } catch { + /* ignore */ + } + } + }); + + itremote('fs.writeFile(path, "", cb) truncates an existing file', async () => { + const fs = require('node:fs'); + const path = require('node:path'); + const file = path.join(require('node:os').tmpdir(), `electron-fswrite-truncate-${Date.now()}`); + try { + fs.writeFileSync(file, 'non-empty contents'); + expect(fs.statSync(file).size).to.be.greaterThan(0); + await new Promise((resolve, reject) => { + fs.writeFile(file, '', (err: Error | null) => (err ? reject(err) : resolve())); + }); + expect(fs.statSync(file).size).to.equal(0); + } finally { + try { + fs.unlinkSync(file); + } catch { + /* ignore */ + } + } + }); + + itremote('fs.writeFile(fd, Buffer.alloc(0), cb) succeeds without closing a caller-owned fd', async () => { + const fs = require('node:fs'); + const path = require('node:path'); + const file = path.join(require('node:os').tmpdir(), `electron-fswrite-user-fd-${Date.now()}`); + const fd = fs.openSync(file, 'w'); + try { + await new Promise((resolve, reject) => { + fs.writeFile(fd, Buffer.alloc(0), (err: Error | null) => (err ? reject(err) : resolve())); + }); + // If writeFile had closed our fd this write would throw EBADF. + fs.writeSync(fd, 'still open'); + fs.closeSync(fd); + expect(fs.readFileSync(file, 'utf-8')).to.equal('still open'); + } finally { + try { + fs.closeSync(fd); + } catch { + /* already closed */ + } + try { + fs.unlinkSync(file); + } catch { + /* ignore */ + } + } + }); + }); + describe('error thrown in renderer process node context', () => { itremote( 'gets emitted as a process uncaughtException event',