Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust's signal stack should be guarded against overflow #69533

Closed
cuviper opened this issue Feb 28, 2020 · 1 comment · Fixed by #69969
Closed

Rust's signal stack should be guarded against overflow #69533

cuviper opened this issue Feb 28, 2020 · 1 comment · Fixed by #69969
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Feb 28, 2020

Rust's runtime installs a SIGSEGV signal handler to distinguish stack overflows from other faults. This requires using an alternate signal stack (sigaltstack), which we create with the default size SIGSTKSZ. That's usually quite small, 8KB or so depending on the target, but it's still plenty for what the SIGSEGV handler is doing. There's currently no guard page like we have on normal stacks.

That alternate stack is shared by all signals for a thread, so if a user installs any other signal handler, and they opt in with the flag SA_ONSTACK, they'll be limited to the same tiny stack. If their handler happens to be more involved and overflows that stack, it may silently clobber other memory. If we had a guard page on the signal stack, that would cause a process abort instead.

This was reported to the Security Response WG with a reproducer in this repo. However, after discussion we decided to treat this as a normal issue for a few reasons:

  • Setting a custom signal handler requires unsafe code.
  • Signal handlers already run in a quite constrained environment, e.g. signal-safety(7).
  • It's unusual to use SA_ONSTACK for signals other than SIGSEGV, and if you choose so, you also bear responsibility to make sure that stack suffices.

Nevertheless, as a defensive measure, it's still a reasonable idea to map a PROT_NONE guard page when Rust is creating its signal stack, so an unexpected overflow will abort rather than silently corrupting memory. For full protection, that would also depend on the guard page not being skipped, but not all targets have stack probes yet (#43241).


Thanks to @eust for reporting this in accordance with our security policy, even though we have decided not to treat this as a security issue at this time.

cc @rust-lang/security

@cuviper cuviper added A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 28, 2020
@Mark-Simulacrum
Copy link
Member

cc @joshtriplett

This is another reason we perhaps shouldn't be registering the signal handler... cc #66809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants