Skip to content

feat: Implement signal delivery for x64 sandbox#15

Open
t-noh wants to merge 3 commits into
lfi-project:masterfrom
t-noh:x64-signal-pr
Open

feat: Implement signal delivery for x64 sandbox#15
t-noh wants to merge 3 commits into
lfi-project:masterfrom
t-noh:x64-signal-pr

Conversation

@t-noh

@t-noh t-noh commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

With this feature, now lfi-runtime can support programs that install custom SIGSEGV/SIGILL handler (e.g., JS runtime) on Linux x86_64. Restoring FP regs is not supported.

This PR requires musl with c28e53
Let me know if you think it is better to force align the unaligned restorer instead of emitting an error.

Comment thread linux/linux.c
.opts = opts,
};

lfi_linux_register_sighandler(engine);

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.

Let's keep this disabled in SYS_MINIMAL mode. The static functions will also need to be gated to avoid being unused function warnings.

@zyedidia zyedidia marked this pull request as ready for review May 20, 2026 21:40
Comment thread linux/linux.c Outdated
Comment thread linux/arch/arm64/sys.c Outdated
Comment thread linux/linux.c Outdated
__builtin_unreachable();
}

if (!arch_forward_signal(lfi_cur_ctx(), sig, si, ucontext)) {

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.

lfi_cur_ctx() will dereference lfi_invoke_info.ctx, which may be null on a thread that is not in a sandbox (signal could arrive at any time). Either that function should be modified, or this call site should additionally check that lfi_invoke_info.ctx is valid before calling that.

@t-noh t-noh May 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've updated the function. I feel like engine should be accessible even without ctx (or LinuxProc or LinuxThread) when engine gets initialized first before setting lfi-Invoke_info.ctx. Is there any reason why lib_engine exists only for libraries?

Comment thread linux/arch/x64/sys.c
sp -= kRedzoneSize; // skip redzone
sp = ROUNDDOWN(sp, 16);
sp -= sizeof(sf);
assert((sp & 15) == 8);

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.

We should probably validate sp here to make sure it is still within the sandbox.

Comment thread linux/arch/x64/sys.c Outdated
Comment thread linux/linux.c Outdated
Comment thread linux/linux.c Outdated
Comment thread linux/arch/x64/sys.c Outdated
Comment thread linux/proc.h
struct LFILinuxEngine *engine;

// Per-signal handler table.
struct SigActionEntry signals[LINUX_NSIG];

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.

I think we need a lock to protect this.

Comment thread linux/linux.c Outdated
@t-noh t-noh force-pushed the x64-signal-pr branch 2 times, most recently from 5829167 to a98f6a4 Compare May 27, 2026 22:14
t-noh added 3 commits June 1, 2026 20:44
Add support for forwarding SIGSEGV/SIGILL to sandbox-registered signal
handlers on Linux x86_64.

* Define x86-64 signal frame types (SigFrame, SigInfo, UContext,
  FPState, SigAction) to arch_types.h

* Register host-level sigaction for SIGSEGV, SIGILL, SIGBUS on linux
  engine init, chaining to prior handlers when not handled

* Implement rt_sigaction and rt_sigreturn syscalls; replace prior stubs

* Add per-process signal handler table (signals[LINUX_NSIG]) to
  LFILinuxProc

* Implement arch_forward_signal: builds a (partial) signal frame on the
  sandbox stack, runs the sandbox handler via lfi_ctx_run, reads back
the modified frame on return, and validates/restores RIP

* Fix lfi_ctx_run to save/restore invoke_info.ctx around lfi_ctx_entry to support nested sandbox entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants