vfs: do not reuse the path of a loaded memfd image

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<int>(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;
   }

src/node_binding.h

@@ -177,7 +177,9 @@ void DLOpenBinary(const v8::FunctionCallbackInfo<v8::Value>& 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

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();
+  }
+});

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);
 `;

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);
+}