bun:ffi: throw when calling a symbol after close() instead of jumping into freed JIT memory

src/jsc/bindings/JSFFIFunction.cpp

@@ -104,6 +104,20 @@ extern "C" void Bun__FFIFunction_setDataPtr(JSC::EncodedJSValue jsValue, void* p
     function->dataPtr = ptr;
 }
 
+// Called from FFI.Function.deinit (ffi.zig) before the per-function TCC state
+// is deleted. Nulls out the native trampoline pointer so that any JS
+// references to this function which are invoked after the owning library is
+// closed will throw a TypeError instead of jumping into freed JIT memory.
+extern "C" void Bun__FFIFunction_setClosed(JSC::EncodedJSValue jsValue)
+{
+    Zig::JSFFIFunction* function = dynamicDowncast<Zig::JSFFIFunction>(JSC::JSValue::decode(jsValue));
+    if (!function)
+        return;
+
+    function->setFunction(nullptr);
+    function->symbolFromDynamicLibrary = nullptr;
+}
+
 extern "C" JSC::EncodedJSValue Bun__CreateFFIFunctionValue(Zig::GlobalObject* globalObject, const ZigString* symbolName, unsigned argCount, Zig::FFIFunction functionPointer, bool addPtrField, void* symbolFromDynamicLibrary)
 {
     if (addPtrField) {
@@ -158,23 +172,25 @@ JSFFIFunction* JSFFIFunction::create(VM& vm, Zig::GlobalObject* globalObject, un
     return function;
 }
 
-#if OS(WINDOWS)
-
 JSC_DEFINE_HOST_FUNCTION(JSFFIFunction::trampoline, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
 {
     const auto* function = uncheckedDowncast<JSFFIFunction>(callFrame->jsCallee());
-    return function->function()(globalObject, callFrame);
+    auto native = function->function();
+    if (!native) [[unlikely]] {
+        auto& vm = JSC::getVM(globalObject);
+        auto scope = DECLARE_THROW_SCOPE(vm);
+        return throwVMTypeError(globalObject, scope, "Cannot call an FFI function after the library has been closed"_s);
+    }
+    return native(globalObject, callFrame);
 }
 
-#endif
-
 JSFFIFunction* JSFFIFunction::createForFFI(VM& vm, Zig::GlobalObject* globalObject, unsigned length, const String& name, CFFIFunction FFIFunction)
 {
-#if OS(WINDOWS)
+    // Always route through the static trampoline so the TinyCC-compiled
+    // function pointer lives in the mutable m_function field instead of being
+    // baked into a NativeExecutable. This lets us safely detach the native
+    // code when the library is closed (see Bun__FFIFunction_setClosed).
     NativeExecutable* executable = vm.getHostFunction(trampoline, ImplementationVisibility::Public, NoIntrinsic, trampoline, nullptr, name);
-#else
-    NativeExecutable* executable = vm.getHostFunction(FFIFunction, ImplementationVisibility::Public, NoIntrinsic, FFIFunction, nullptr, name);
-#endif
     Structure* structure = globalObject->FFIFunctionStructure();
     JSFFIFunction* function = new (NotNull, allocateCell<JSFFIFunction>(vm)) JSFFIFunction(vm, executable, globalObject, structure, reinterpret_cast<CFFIFunction>(WTF::move(FFIFunction)));
     function->finishCreation(vm, executable, length, name);

src/jsc/bindings/JSFFIFunction.h

@@ -79,13 +79,14 @@ class JSFFIFunction final : public JSC::JSFunction {
     }
 
     const CFFIFunction function() const { return m_function; }
+    void setFunction(CFFIFunction function) { m_function = function; }
 
-#if OS(WINDOWS)
-
+    // All calls to FFI functions created via createForFFI() route through this
+    // trampoline, which dispatches to m_function. This indirection lets us
+    // null out m_function when the owning library is closed so that calling a
+    // symbol after close() throws instead of jumping into freed JIT memory.
     static JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES trampoline(JSGlobalObject* globalObject, CallFrame* callFrame);
 
-#endif
-
     void* dataPtr;
     void* symbolFromDynamicLibrary { nullptr };
 

src/runtime/ffi/ffi.zig

@@ -544,8 +544,12 @@ pub const FFI = struct {
     const SymbolsMap = struct {
         map: bun.StringArrayHashMapUnmanaged(Function) = .{},
         pub fn deinit(this: *SymbolsMap) void {
-            for (this.map.keys()) |key| {
-                bun.default_allocator.free(@constCast(key));
+            // Each map key aliases the Function's base_name allocation, which
+            // Function.deinit frees along with the per-symbol TCC state,
+            // arg_types, and (on the dlopen/linkSymbols/cc paths) the
+            // protect()ed JSFFIFunction root.
+            for (this.map.values()) |*value| {
+                value.deinitWithoutGlobal();
             }
             this.map.clearAndFree(bun.default_allocator);
         }
@@ -810,6 +814,9 @@ pub const FFI = struct {
                         function.symbol_from_dynamic_library,
                     );
                     compiled.js_function = cb;
+                    // Rooted so Function.deinit can safely detach it on
+                    // close() even if it was removed from the symbols object.
+                    cb.protect();
                     obj.put(globalThis, &str, cb);
                 },
             }
@@ -1110,8 +1117,7 @@ pub const FFI = struct {
                 const resolved_symbol = dylib.lookup(*anyopaque, function_name) orelse {
                     const ret = global.toInvalidArguments("Symbol \"{s}\" not found in \"{s}\"", .{ bun.asByteSlice(function_name), name });
                     for (symbols.values()) |*value| {
-                        bun.default_allocator.free(@constCast(bun.asByteSlice(value.base_name.?)));
-                        value.arg_types.clearAndFree(bun.default_allocator);
+                        value.deinit(global);
                     }
                     symbols.clearAndFree(bun.default_allocator);
                     dylib.close();
@@ -1136,11 +1142,10 @@ pub const FFI = struct {
             };
             switch (function.step) {
                 .failed => |err| {
-                    defer for (symbols.values()) |*other_function| {
-                        other_function.deinit(global);
-                    };
-
                     const res = ZigString.init(err.msg).toErrorInstance(global);
+                    for (symbols.values()) |*other_function| {
+                        other_function.deinit(global);
+                    }
                     symbols.clearAndFree(bun.default_allocator);
                     dylib.close();
                     return res;
@@ -1164,6 +1169,9 @@ pub const FFI = struct {
                         function.symbol_from_dynamic_library,
                     );
                     compiled.js_function = cb;
+                    // Rooted so Function.deinit can safely detach it on
+                    // close() even if it was removed from the symbols object.
+                    cb.protect();
                     obj.put(global, &str, cb);
                 },
             }
@@ -1223,8 +1231,7 @@ pub const FFI = struct {
             if (function.symbol_from_dynamic_library == null) {
                 const ret = global.toInvalidArguments("Symbol \"{s}\" is missing a \"ptr\" field. When using linkSymbols() or CFunction(), you must provide a \"ptr\" field with the memory address of the native function.", .{bun.asByteSlice(function_name)});
                 for (symbols.values()) |*value| {
-                    allocator.free(@constCast(bun.asByteSlice(value.base_name.?)));
-                    value.arg_types.clearAndFree(allocator);
+                    value.deinit(global);
                 }
                 symbols.clearAndFree(allocator);
                 return ret;
@@ -1243,20 +1250,16 @@ pub const FFI = struct {
             };
             switch (function.step) {
                 .failed => |err| {
+                    const res = ZigString.init(err.msg).toErrorInstance(global);
                     for (symbols.values()) |*value| {
-                        allocator.free(@constCast(bun.asByteSlice(value.base_name.?)));
-                        value.arg_types.clearAndFree(allocator);
+                        value.deinit(global);
                     }
-
-                    const res = ZigString.init(err.msg).toErrorInstance(global);
-                    function.deinit(global);
                     symbols.clearAndFree(allocator);
                     return res;
                 },
                 .pending => {
                     for (symbols.values()) |*value| {
-                        allocator.free(@constCast(bun.asByteSlice(value.base_name.?)));
-                        value.arg_types.clearAndFree(allocator);
+                        value.deinit(global);
                     }
                     symbols.clearAndFree(allocator);
                     return ZigString.static("Failed to compile (nothing happend!)").toErrorInstance(global);
@@ -1273,6 +1276,9 @@ pub const FFI = struct {
                         function.symbol_from_dynamic_library,
                     );
                     compiled.js_function = cb;
+                    // Rooted so Function.deinit can safely detach it on
+                    // close() even if it was removed from the symbols object.
+                    cb.protect();
 
                     obj.put(global, name, cb);
                 },
@@ -1459,8 +1465,13 @@ pub const FFI = struct {
         }
 
         extern "c" fn FFICallbackFunctionWrapper_destroy(*anyopaque) void;
+        extern "c" fn Bun__FFIFunction_setClosed(JSValue) void;
+
+        pub fn deinit(val: *Function, _: *jsc.JSGlobalObject) void {
+            val.deinitWithoutGlobal();
+        }
 
-        pub fn deinit(val: *Function, globalThis: *jsc.JSGlobalObject) void {
+        pub fn deinitWithoutGlobal(val: *Function) void {
             jsc.markBinding(@src());
 
             if (val.base_name) |base_name| {
@@ -1471,14 +1482,24 @@ pub const FFI = struct {
 
             val.arg_types.clearAndFree(val.allocator);
 
-            if (val.state) |state| {
-                state.deinit();
-                val.state = null;
-            }
-
             if (val.step == .compiled) {
                 if (val.step.compiled.js_function != .zero) {
-                    _ = globalThis;
+                    // On the dlopen/linkSymbols/cc paths, js_function is the
+                    // JSFFIFunction we created for this symbol (rooted via
+                    // protect() at creation time) whose native trampoline
+                    // lives in the TCC state about to be freed. Detach it so
+                    // calling it throws instead of jumping into freed JIT
+                    // memory, then release the GC root.
+                    //
+                    // On the JSCallback path (ffi_callback_function_wrapper
+                    // != null), js_function is the user's callback which we
+                    // merely borrowed — it is rooted via the wrapper's
+                    // Strong<JSFunction> and may itself be an unrelated FFI
+                    // symbol, so it must not be detached here.
+                    if (val.step.compiled.ffi_callback_function_wrapper == null) {
+                        Bun__FFIFunction_setClosed(val.step.compiled.js_function);
+                        val.step.compiled.js_function.unprotect();
+                    }
                     val.step.compiled.js_function = .zero;
                 }
 
@@ -1488,6 +1509,11 @@ pub const FFI = struct {
                 }
             }
 
+            if (val.state) |state| {
+                state.deinit();
+                val.state = null;
+            }
+
             if (val.step == .failed and val.step.failed.allocated) {
                 val.allocator.free(val.step.failed.msg);
             }

test/js/bun/ffi/ffi.test.js

@@ -1,13 +1,14 @@
 import { afterAll, describe, expect, it } from "bun:test";
 import { existsSync } from "fs";
-import { isGlibcVersionAtLeast } from "harness";
+import { isArm64, isGlibcVersionAtLeast, isWindows } from "harness";
 import { platform } from "os";
 
 import {
   dlopen as _dlopen,
   CFunction,
   CString,
   JSCallback,
+  linkSymbols,
   ptr,
   read,
   suffix,
@@ -970,3 +971,138 @@ describe.if(!!libPath)("can open more than 63 symbols via", () => {
     });
   }
 });
+
+// TinyCC (which compiles the FFI trampolines) is disabled on Windows ARM64.
+describe.skipIf(isWindows && isArm64)("calling an FFI symbol after close()", () => {
+  // Regression test: calling a captured FFI symbol after its library has been
+  // closed must throw instead of jumping into the freed TinyCC JIT pages.
+  it("linkSymbols: throws instead of calling freed code", () => {
+    const cb = new JSCallback(x => x + 1, { args: ["i32"], returns: "i32" });
+    try {
+      const lib = linkSymbols({
+        inc: { args: ["i32"], returns: "i32", ptr: cb.ptr },
+      });
+      const inc = lib.symbols.inc;
+      const native = inc.native;
+      expect(inc(41)).toBe(42);
+      expect(native(41)).toBe(42);
+
+      lib.close();
+
+      expect(() => inc(1)).toThrow(TypeError);
+      expect(() => inc(1)).toThrow("Cannot call an FFI function after the library has been closed");
+      expect(() => native(1)).toThrow(TypeError);
+
+      // close() is idempotent
+      lib.close();
+      expect(() => inc(1)).toThrow(TypeError);
+    } finally {
+      cb.close();
+    }
+  });
+
+  it("linkSymbols: zero-arg function throws instead of calling freed code", () => {
+    const cb = new JSCallback(() => 7, { returns: "i32" });
+    try {
+      const lib = linkSymbols({
+        seven: { returns: "i32", ptr: cb.ptr },
+      });
+      // zero-arg functions are not wrapped by FFIBuilder, so the symbol IS
+      // the native JSFFIFunction itself.
+      const seven = lib.symbols.seven;
+      expect(seven()).toBe(7);
+
+      lib.close();
+
+      expect(() => seven()).toThrow(TypeError);
+      expect(() => seven()).toThrow("Cannot call an FFI function after the library has been closed");
+    } finally {
+      cb.close();
+    }
+  });
+
+  it("close() after symbol was deleted from lib.symbols and GC'd does not read a freed cell", () => {
+    // compiled.js_function is stored in the Zig heap; it must be rooted so
+    // that close() can safely detach it even if the user removed it from
+    // the (mutable) symbols object and a GC ran in between.
+    const cb = new JSCallback(() => 7, { returns: "i32" });
+    try {
+      const lib = linkSymbols({ seven: { returns: "i32", ptr: cb.ptr } });
+      const seven = lib.symbols.seven;
+      expect(seven()).toBe(7);
+
+      delete lib.symbols.seven;
+      Bun.gc(true);
+
+      // Must not read a freed/reused GC cell.
+      lib.close();
+
+      // `seven` was kept alive by our local reference and must have been
+      // detached by close().
+      expect(() => seven()).toThrow(TypeError);
+    } finally {
+      cb.close();
+    }
+  });
+
+  it("mid-loop failure unroots and detaches already-compiled symbols", () => {
+    // If linkSymbols fails on symbol N after symbols 0..N-1 have been
+    // compiled + protected, the cleanup path must run Function.deinit on
+    // each earlier symbol (unprotect + setClosed) rather than only freeing
+    // base_name/arg_types. Otherwise each earlier JSFFIFunction becomes a
+    // permanent GC root and its TCC state leaks.
+    const cb = new JSCallback(() => 7, { returns: "i32" });
+    try {
+      let captured;
+      expect(() => {
+        captured = linkSymbols({
+          good: { returns: "i32", ptr: cb.ptr },
+          // missing `ptr` triggers the mid-loop error after `good` compiled
+          bad: { returns: "i32" },
+        });
+      }).toThrow(/"bad".*ptr/);
+      expect(captured).toBeUndefined();
+      // No observable side effects; primary regression signal is no ASAN
+      // leak report and (once the process exits) a balanced gcProtect table.
+    } finally {
+      cb.close();
+    }
+  });
+
+  it("JSCallback.close() does not detach an FFI symbol passed as the callback", () => {
+    // On the JSCallback path, Function.step.compiled.js_function holds the
+    // user's callback (which may itself be a JSFFIFunction from another
+    // still-open library). Closing the JSCallback must not null that
+    // symbol's trampoline.
+    const target = new JSCallback(() => 7, { returns: "i32" });
+    try {
+      const lib = linkSymbols({ seven: { returns: "i32", ptr: target.ptr } });
+      const seven = lib.symbols.seven; // raw JSFFIFunction
+      expect(seven()).toBe(7);
+
+      const wrapper = new JSCallback(seven, { returns: "i32" });
+      wrapper.close();
+
+      // lib is still open; seven must still work.
+      expect(seven()).toBe(7);
+
+      lib.close();
+      expect(() => seven()).toThrow(TypeError);
+    } finally {
+      target.close();
+    }
+  });
+
+  it.if(!!libPath)("dlopen: throws instead of calling freed code", () => {
+    const lib = _dlopen(libPath, { strlen: { args: ["ptr"], returns: "usize" } });
+    const strlen = lib.symbols.strlen;
+    const native = strlen.native;
+    expect(strlen(Buffer.from("bun\0"))).toBe(3n);
+
+    lib.close();
+
+    expect(() => strlen(Buffer.from("bun\0"))).toThrow(TypeError);
+    expect(() => strlen(Buffer.from("bun\0"))).toThrow("Cannot call an FFI function after the library has been closed");
+    expect(() => native(Buffer.from("bun\0"))).toThrow(TypeError);
+  });
+});