Skip to content

ffi: reject fast calls after library close - #64860

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:ffi-closed-lib-cal-sigsegv-3
Aug 2, 2026
Merged

ffi: reject fast calls after library close#64860
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:ffi-closed-lib-cal-sigsegv-3

Conversation

@trivikr

@trivikr trivikr commented Jul 31, 2026

Copy link
Copy Markdown
Member

Fixes: #64854

Optimized FFI Fast API calls bypass DynamicLibrary::InvokeFunction() and enter the generated trampoline directly. After DynamicLibrary::close() unloads the library, optimized code could call the stale symbol address and crash with SIGSEGV.

This adds a closed-state check to the AArch64 and SysV x64 fast trampolines. The hot path continues to the native symbol when the library is open. The cold path schedules ERR_FFI_LIBRARY_CLOSED and returns without entering the unloaded library.


Assisted-by: codex:gpt-5.6-sol

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 31, 2026
Optimized Fast API calls bypass InvokeFunction and can jump directly to
a symbol after DynamicLibrary::close() unloads its library.

Check the function's closed state in the AArch64 and SysV x64
trampolines before entering the target. If the library is closed,
schedule ERR_FFI_LIBRARY_CLOSED and return without calling the symbol.

Keep the JavaScript guard on platforms without a native trampoline guard
and for signatures that already require argument conversion or
validation. This keeps raw scalar fast calls close to their original
performance on supported platforms.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the ffi-closed-lib-cal-sigsegv-3 branch from 8e91308 to 3b1b100 Compare July 31, 2026 04:27
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.17%. Comparing base (c8e2a82) to head (14d6323).
⚠️ Report is 39 commits behind head on main.

Files with missing lines Patch % Lines
src/ffi/fast.cc 85.71% 1 Missing and 1 partial ⚠️
src/ffi/platforms/arm64.cc 89.47% 0 Missing and 2 partials ⚠️
lib/internal/ffi/fast-api.js 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64860      +/-   ##
==========================================
+ Coverage   90.16%   90.17%   +0.01%     
==========================================
  Files         746      746              
  Lines      242760   242847      +87     
  Branches    45765    45768       +3     
==========================================
+ Hits       218875   218987     +112     
+ Misses      15375    15341      -34     
- Partials     8510     8519       +9     
Files with missing lines Coverage Δ
lib/ffi.js 95.23% <100.00%> (+0.31%) ⬆️
lib/internal/errors.js 97.90% <100.00%> (+<0.01%) ⬆️
src/ffi/fast.h 100.00% <100.00%> (ø)
src/node_ffi.cc 70.45% <100.00%> (-0.18%) ⬇️
lib/internal/ffi/fast-api.js 92.32% <96.96%> (+1.25%) ⬆️
src/ffi/fast.cc 68.71% <85.71%> (+0.95%) ⬆️
src/ffi/platforms/arm64.cc 85.40% <89.47%> (+0.88%) ⬆️

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 31, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@trivikr
trivikr marked this pull request as draft July 31, 2026 20:20
@trivikr

trivikr commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Moved to draft since CI is failing on Linux on ARM

not ok 8 ffi/test-ffi-dynamic-library
  ---
  duration_ms: 509.99000
  severity: crashed
  exitcode: -6
  stack: |-
    (node:3793798) ExperimentalWarning: FFI is an experimental feature and might change at any time
    (Use `node --trace-warnings ...` to show where the warning was created)
    FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
    ----- Native stack trace -----
    
     1: 0xc9ccdcdb50cc node::DumpNativeBacktrace(_IO_FILE*) [out/Debug/node]
     2: 0xc9ccdcf2bad8 node::OnFatalError(char const*, char const*) [out/Debug/node]
     3: 0xc9ccdd3bb228  [out/Debug/node]
     4: 0xc9ccdd7e04e8  [out/Debug/node]
     5: 0xc9ccdd3b1794  [out/Debug/node]
     6: 0xc9ccddf78f80  [out/Debug/node]
     7: 0xc9ccdd878fc8  [out/Debug/node]
     8: 0xc9ccdd3f50f4 v8::String::NewFromOneByte(v8::Isolate*, unsigned char const*, v8::NewStringType, int) [out/Debug/node]
     9: 0xc9ccdccfd20c node::OneByteString(v8::Isolate*, char const*, int, v8::NewStringType) [out/Debug/node]
    10: 0xc9ccdcd2cd20 v8::Local<v8::String> node::FIXED_ONE_BYTE_STRING<23ul>(v8::Isolate*, char const (&) [23ul]) requires ((23ul)>(0)) [out/Debug/node]
    11: 0xc9ccdd2104b8 v8::Local<v8::Object> node::ERR_FFI_LIBRARY_CLOSED<>(v8::Isolate*, std::basic_string_view<char, std::char_traits<char> >) [out/Debug/node]
    12: 0xc9ccdd21043c node::THROW_ERR_FFI_LIBRARY_CLOSED(v8::Isolate*) [out/Debug/node]
    13: 0xc9ccdd221ef0 node_ffi_fast_library_closed [out/Debug/node]
    14: 0xebea4cb1b044 
    FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
    ----- Native stack trace -----
    
     1: 0xc9ccdcdb50cc node::DumpNativeBacktrace(_IO_FILE*) [out/Debug/node]
     2: 0xc9ccdcf2bad8 node::OnFatalError(char const*, char const*) [out/Debug/node]
     3: 0xc9ccdd3bb228  [out/Debug/node]
     4: 0xc9ccdd7e04e8  [out/Debug/node]
     5: 0xc9ccdd3bc518 v8::EscapableHandleScopeBase::EscapableHandleScopeBase(v8::Isolate*) [out/Debug/node]
     6: 0xc9ccdccf9a04 v8::EscapableHandleScope::EscapableHandleScope(v8::Isolate*) [out/Debug/node]
     7: 0xc9ccdcf2a08c node::GetCurrentStackTrace(v8::Isolate*, int) [out/Debug/node]
     8: 0xc9ccdcdb51f0 node::DumpJavaScriptBacktrace(_IO_FILE*) [out/Debug/node]
     9: 0xc9ccdcf2bae4 node::OnFatalError(char const*, char const*) [out/Debug/node]
    10: 0xc9ccdd3bb228  [out/Debug/node]
    11: 0xc9ccdd7e04e8  [out/Debug/node]
    12: 0xc9ccdd3b1794  [out/Debug/node]
    13: 0xc9ccddf78f80  [out/Debug/node]
    14: 0xc9ccdd878fc8  [out/Debug/node]
    15: 0xc9ccdd3f50f4 v8::String::NewFromOneByte(v8::Isolate*, unsigned char const*, v8::NewStringType, int) [out/Debug/node]
    16: 0xc9ccdccfd20c node::OneByteString(v8::Isolate*, char const*, int, v8::NewStringType) [out/Debug/node]
    17: 0xc9ccdcd2cd20 v8::Local<v8::String> node::FIXED_ONE_BYTE_STRING<23ul>(v8::Isolate*, char const (&) [23ul]) requires ((23ul)>(0)) [out/Debug/node]
    18: 0xc9ccdd2104b8 v8::Local<v8::Object> node::ERR_FFI_LIBRARY_CLOSED<>(v8::Isolate*, std::basic_string_view<char, std::char_traits<char> >) [out/Debug/node]
    19: 0xc9ccdd21043c node::THROW_ERR_FFI_LIBRARY_CLOSED(v8::Isolate*) [out/Debug/node]
    20: 0xc9ccdd221ef0 node_ffi_fast_library_closed [out/Debug/node]
    21: 0xebea4cb1b044 
  ...

CI: https://ci.nodejs.org/job/node-test-commit-arm-debug/24671/

Fast API calls run without an entered HandleScope, and the generated
trampolines call node_ffi_fast_library_closed() directly. Constructing
ERR_FFI_LIBRARY_CLOSED allocates handles, which aborted debug builds
with "Cannot create a handle without a HandleScope".

Open a HandleScope in the helper before throwing. The scheduled
exception is stored on the isolate, so it outlives the scope. Apply the
same fix to node_ffi_fast_buffer_data(), which had the same latent bug
on its throw path.
@trivikr

trivikr commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr
trivikr marked this pull request as ready for review July 31, 2026 22:50
@trivikr

This comment was marked as outdated.

@trivikr trivikr added the ffi Issues and PRs related to experimental Foreign Function Interface support. label Aug 1, 2026
@trivikr
trivikr requested a review from ShogunPanda August 2, 2026 02:48
@trivikr trivikr added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 2, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 2, 2026
@nodejs-github-bot
nodejs-github-bot merged commit c55eb4a into nodejs:main Aug 2, 2026
95 of 96 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in c55eb4a

aduh95 pushed a commit that referenced this pull request Aug 3, 2026
Optimized Fast API calls bypass InvokeFunction and can jump directly to
a symbol after DynamicLibrary::close() unloads its library.

Check the function's closed state in the AArch64 and SysV x64
trampolines before entering the target. If the library is closed,
schedule ERR_FFI_LIBRARY_CLOSED and return without calling the symbol.

Keep the JavaScript guard on platforms without a native trampoline guard
and for signatures that already require argument conversion or
validation. This keeps raw scalar fast calls close to their original
performance on supported platforms.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
PR-URL: #64860
Fixes: #64854
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. ffi Issues and PRs related to experimental Foreign Function Interface support. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: closed library still callable via fast path, SIGSEGV instead of ERR_FFI_LIBRARY_CLOSED

5 participants