diff --git a/src/node_binding.cc b/src/node_binding.cc index 3c7dee9bb33..b8a1a6f64c4 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -476,6 +476,27 @@ bool WriteAllToFd(int fd, const char* data, size_t len) { int NodeMemfdCreate(const char* name, unsigned int flags) { return static_cast(syscall(SYS_memfd_create, name, flags)); } + +// The dynamic loader knows a loaded object by the path it was opened with, and +// hands back the object already loaded under a path instead of opening the +// file again. An image is loaded through /proc/self/fd/N and its fd is closed +// right after, so the kernel can hand N to the next image while the first is +// still loaded. So ask the loader, and move to a higher number while the path +// is taken. Holding the fd from here through the load keeps anyone else from +// taking the path in between, and an object that has really been unloaded gives +// its number back, so only images loaded at the same time compete for numbers. +int MoveFdToFreeImagePath(int fd) { + while (true) { + const std::string path = "/proc/self/fd/" + std::to_string(fd); + void* loaded = dlopen(path.c_str(), RTLD_LAZY | RTLD_NOLOAD); + if (loaded == nullptr) return fd; + dlclose(loaded); + const int moved = fcntl(fd, F_DUPFD_CLOEXEC, fd + 1); + close(fd); + if (moved == -1) return -1; + fd = moved; + } +} #endif // __linux__ #else // _WIN32 @@ -636,7 +657,8 @@ bool AddonImage::Materialize(const char* data, size_t len) { // reaches the filesystem, so there is no temp file to secure, unlink, or // leak, and no noexec-mount problem. Request an executable memfd (MFD_EXEC, // Linux 6.3+); retry without it on older kernels that reject the flag, then - // fall back to a temp file if memfd is unavailable entirely. + // fall back to a temp file if memfd is unavailable entirely, or if so many + // images are loaded that no fd number is left to load another through. #ifndef MFD_CLOEXEC #define MFD_CLOEXEC 0x0001U #endif @@ -654,6 +676,9 @@ bool AddonImage::Materialize(const char* data, size_t len) { fd_ = -1; return false; } + fd_ = MoveFdToFreeImagePath(fd_); + } + if (fd_ != -1) { path_ = "/proc/self/fd/" + std::to_string(fd_); return true; } diff --git a/src/node_binding.h b/src/node_binding.h index ee30117d206..0a8e3b4a120 100644 --- a/src/node_binding.h +++ b/src/node_binding.h @@ -177,7 +177,9 @@ void DLOpenBinary(const v8::FunctionCallbackInfo& args); // dlopen()/LoadLibrary() can load, with the smallest, most private on-disk // footprint each platform allows: // Linux: an anonymous in-memory memfd, loaded via /proc/self/fd/N - -// the bytes never touch the filesystem. +// the bytes never touch the filesystem. The fd is closed right +// after the load, and a number whose path still names a loaded +// object is skipped (the loader would return that object). // other POSIX: a 0700 mkdtemp() directory plus an O_EXCL|O_NOFOLLOW file, // unlink()ed right after the load (the mapping keeps it alive). // Windows: a temp file, written and closed before the load because the diff --git a/test/ffi/test-ffi-vfs.js b/test/ffi/test-ffi-vfs.js index 37926e9c3dd..9ea319cc240 100644 --- a/test/ffi/test-ffi-vfs.js +++ b/test/ffi/test-ffi-vfs.js @@ -3,6 +3,7 @@ const common = require('../common'); common.skipIfFFIMissing(); const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); @@ -46,6 +47,58 @@ test('ffi.dlopen() loads a library from a mounted VFS', () => { } }); +test('library images leave no file descriptor behind', { skip: !common.isLinux }, () => { + const images = () => fs.readdirSync('/proc/self/fd').filter((fd) => { + try { + return fs.readlinkSync(`/proc/self/fd/${fd}`).startsWith('/memfd:node-addon'); + } catch { + return false; + } + }); + + const { lib, functions } = ffi.dlopen(virtualPath, { + add_i32: fixtureSymbols.add_i32, + }); + try { + assert.deepStrictEqual(images(), []); + assert.strictEqual(functions.add_i32(40, 2), 42); + } finally { + lib.close(); + } + assert.deepStrictEqual(images(), []); +}); + +test('closed library images give their fd numbers back', { skip: !common.isLinux }, () => { + // More loads than the limit allows fds, one at a time. There is no usable + // temporary directory, so this only works if each stays an in-memory image. + const limit = 128; + const child = ` + const assert = require('node:assert'); + const fs = require('node:fs'); + const path = require('node:path'); + const ffi = require('node:ffi'); + const vfs = require('node:vfs'); + const myVfs = vfs.create(); + myVfs.writeFileSync('/lib.so', fs.readFileSync(${JSON.stringify(libraryPath)})); + const libPath = path.join(myVfs.mount(), 'lib.so'); + for (let i = 0; i < ${limit * 4}; i++) { + const { lib, functions } = ffi.dlopen(libPath, { + add_i32: ${JSON.stringify(fixtureSymbols.add_i32)}, + }); + assert.strictEqual(functions.add_i32(40, 2), 42); + lib.close(); + } + `; + const { status, stderr } = spawnSync('/bin/sh', [ + '-c', `ulimit -n ${limit} && exec "$0" "$@"`, + process.execPath, '--disable-warning=ExperimentalWarning', '-e', child, + ], { + encoding: 'utf8', + env: { ...process.env, TMPDIR: path.join(os.tmpdir(), 'no-such-directory') }, + }); + assert.strictEqual(status, 0, stderr); +}); + test('new ffi.DynamicLibrary() loads from a mounted VFS', () => { const lib = new ffi.DynamicLibrary(virtualPath); @@ -92,3 +145,57 @@ test('a library loaded from a VFS outlives the mount', () => { lib.close(); } }); + +const addonPath = (name) => path.join( + __dirname, '..', 'addons', name, 'build', common.buildType, 'binding.node'); + +test('an addon loaded after a VFS library is its own module', (t) => { + const helloPath = addonPath('hello-world'); + if (!fs.existsSync(helloPath)) { + t.skip('the hello-world addon is not built'); + return; + } + const seqVfs = vfs.create(); + seqVfs.writeFileSync(`/${libraryName}`, fs.readFileSync(libraryPath)); + seqVfs.writeFileSync('/hello.node', fs.readFileSync(helloPath)); + const seqMount = seqVfs.mount(); + const { lib, functions } = ffi.dlopen(path.join(seqMount, libraryName), { + add_i32: fixtureSymbols.add_i32, + }); + + try { + // Resolved to the library, the addon would fail with "Module did not + // self-register". + const addon = require(path.join(seqMount, 'hello.node')); + assert.strictEqual(addon.hello(), 'world'); + assert.strictEqual(functions.add_i32(20, 22), 42); + } finally { + lib.close(); + seqVfs.unmount(); + } +}); + +test('a VFS library loaded after an addon is its own library', (t) => { + const functionPath = addonPath('hello-world-function-export'); + if (!fs.existsSync(functionPath)) { + t.skip('the hello-world-function-export addon is not built'); + return; + } + const seqVfs = vfs.create(); + seqVfs.writeFileSync('/function.node', fs.readFileSync(functionPath)); + seqVfs.writeFileSync(`/${libraryName}`, fs.readFileSync(libraryPath)); + const seqMount = seqVfs.mount(); + const addon = require(path.join(seqMount, 'function.node')); + assert.strictEqual(addon(), 'world'); + + // Resolved to the addon, the library would have no add_i32 to look up. + const { lib, functions } = ffi.dlopen(path.join(seqMount, libraryName), { + add_i32: fixtureSymbols.add_i32, + }); + try { + assert.strictEqual(functions.add_i32(40, 2), 42); + } finally { + lib.close(); + seqVfs.unmount(); + } +}); diff --git a/test/parallel/test-dlopen-binary-image-cleanup.js b/test/parallel/test-dlopen-binary-image-cleanup.js index dabaeabaa0c..93a6cf7521b 100644 --- a/test/parallel/test-dlopen-binary-image-cleanup.js +++ b/test/parallel/test-dlopen-binary-image-cleanup.js @@ -7,9 +7,8 @@ // this checks both halves of the contract: // // Linux: an anonymous memfd loaded through /proc/self/fd - nothing ever -// reaches the filesystem, and AfterOpen() closes the descriptor -// once the load owns its mapping, so repeated loads must not -// accumulate open descriptors. +// reaches the filesystem, and the fd is closed right after the +// load, whether or not it worked. // other POSIX: a mkdtemp() directory unlinked and rmdir()ed right after the // load, so nothing is left even while the process runs. // Windows: the loader maps the file by path for the DLL's lifetime, so @@ -45,27 +44,48 @@ const child = ` // the load can only come from the bytes and their materialized image. const virtualPath = ${JSON.stringify(path.join(addonPath, '..', 'nowhere', 'binding.node'))}; - // On Linux the image is a descriptor rather than a file, so count them: each - // load must hand its fd to the mapping and close it, leaving no growth. + // On Linux the image is a memfd rather than a file, so count those: no load + // may keep one. const fdDir = '/proc/self/fd'; - const countFds = () => { - try { return fs.readdirSync(fdDir).length; } catch { return -1; } + const imageFds = () => { + let fds; + try { fds = fs.readdirSync(fdDir); } catch { return null; } + return fds.filter((fd) => { + try { + return fs.readlinkSync(\`\${fdDir}/\${fd}\`).startsWith('/memfd:node-addon'); + } catch { return false; } + }); }; - const before = countFds(); // Load repeatedly: each load materializes its own image, so a leak of an - // image, a descriptor or a retained handle shows up as growth. - for (let i = 0; i < 5; i++) { + // image or a retained handle shows up as growth. + const loads = 5; + for (let i = 0; i < loads; i++) { const m = { exports: {} }; // flags undefined: keep the default dlopen(2) mode - 0 is not valid // everywhere (glibc rejects it with EINVAL). dlopenBinary(m, virtualPath, undefined, bytes); if (m.exports.hello() !== 'world') throw new Error('addon did not load'); } + const afterLoads = imageFds(); - const after = countFds(); - if (before !== -1 && after > before) { - throw new Error(\`descriptor leak: \${before} -> \${after} after 5 loads\`); + for (let i = 0; i < loads; i++) { + try { + dlopenBinary({ exports: {} }, virtualPath, undefined, Buffer.from('nope')); + throw new Error('loading garbage bytes did not fail'); + } catch (err) { + if (err.code !== 'ERR_DLOPEN_FAILED') throw err; + } + } + const afterFailures = imageFds(); + + if (afterLoads !== null) { + if (afterLoads.length !== 0) { + throw new Error(\`loads kept images: \${afterLoads}\`); + } + if (afterFailures.length !== 0) { + throw new Error(\`failed loads kept images: \${afterFailures}\`); + } } process.exit(0); `; diff --git a/test/parallel/test-vfs-addon-sequence.js b/test/parallel/test-vfs-addon-sequence.js new file mode 100644 index 00000000000..cede831c880 --- /dev/null +++ b/test/parallel/test-vfs-addon-sequence.js @@ -0,0 +1,60 @@ +// Flags: --disable-warning=ExperimentalWarning +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); +const vfs = require('node:vfs'); + +const addonPath = (name) => path.join( + __dirname, '..', 'addons', name, 'build', common.buildType, 'binding.node'); +const helloPath = addonPath('hello-world'); +const functionPath = addonPath('hello-world-function-export'); +if (!fs.existsSync(helloPath) || !fs.existsSync(functionPath)) { + common.skip('the hello-world addons are not built'); +} + +const myVfs = vfs.create(); +myVfs.writeFileSync('/hello.node', fs.readFileSync(helloPath)); +myVfs.writeFileSync('/function.node', fs.readFileSync(functionPath)); +const mountPoint = myVfs.mount(); + +const hello = require(path.join(mountPoint, 'hello.node')); +assert.strictEqual(typeof hello, 'object'); +assert.strictEqual(hello.hello(), 'world'); + +// hello-world-function-export replaces its exports with a function. Resolved +// to the first image, this would be another { hello } object instead. +const fn = require(path.join(mountPoint, 'function.node')); +assert.strictEqual(typeof fn, 'function'); +assert.strictEqual(fn(), 'world'); +assert.notStrictEqual(fn, hello); + +myVfs.unmount(); + +// An addon on the main thread stays loaded and keeps the path it was loaded +// through, so enough of them leave no fd number to load another through. Loading +// has to carry on regardless, through the temp-file fallback. +if (common.isLinux) { + const limit = 128; + const child = ` + const assert = require('node:assert'); + const fs = require('node:fs'); + const path = require('node:path'); + const vfs = require('node:vfs'); + const myVfs = vfs.create(); + myVfs.writeFileSync('/function.node', fs.readFileSync(${JSON.stringify(functionPath)})); + const mountPoint = myVfs.mount(); + for (let i = 0; i < ${limit * 2}; i++) { + const addonPath = path.join(mountPoint, 'function.node'); + assert.strictEqual(require(addonPath)(), 'world'); + delete require.cache[addonPath]; + } + `; + const { status, stderr } = spawnSync('/bin/sh', [ + '-c', `ulimit -n ${limit} && exec "$0" "$@"`, + process.execPath, '--disable-warning=ExperimentalWarning', '-e', child, + ], { encoding: 'utf8' }); + assert.strictEqual(status, 0, stderr); +}