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

Use a range to identify SIGSEGV in stack guards #47912

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jan 31, 2018

Previously, the guard::init() and guard::current() functions were
returning a usize address representing the top of the stack guard,
respectively for the main thread and for spawned threads. The SIGSEGV
handler on unix targets checked if a fault was within one page below that
address, if so reporting it as a stack overflow.

Now unix targets report a Range<usize> representing the guard memory,
so it can cover arbitrary guard sizes. Non-unix targets which always
return None for guards now do so with Option<!>, so they don't pay any
overhead.

For linux-gnu in particular, the previous guard upper-bound was
stackaddr + guardsize, as the protected memory was inside the stack.
This was a glibc bug, and starting from 2.27 they are moving the guard
past the end of the stack. However, there's no simple way for us to know
where the guard page actually lies, so now we declare it as the whole range
of stackaddr ± guardsize, and any fault therein will be called a stack
overflow. This fixes #47863.

Previously, the `guard::init()` and `guard::current()` functions were
returning a `usize` address representing the top of the stack guard,
respectively for the main thread and for spawned threads.  The `SIGSEGV`
handler on `unix` targets checked if a fault was within one page below
that address, if so reporting it as a stack overflow.

Now `unix` targets report a `Range<usize>` representing the guard
memory, so it can cover arbitrary guard sizes.  Non-`unix` targets which
always return `None` for guards now do so with `Option<!>`, so they
don't pay any overhead.

For `linux-gnu` in particular, the previous guard upper-bound was
`stackaddr + guardsize`, as the protected memory was *inside* the stack.
This was a glibc bug, and starting from 2.27 they are moving the guard
*past* the end of the stack.  However, there's no simple way for us to
know where the guard page actually lies, so now we declare it as the
whole range of `stackaddr ± guardsize`, and any fault therein will be
called a stack overflow.  This fixes rust-lang#47863.
@rust-highfive
Copy link
Collaborator

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

@cuviper
Copy link
Member Author

cuviper commented Jan 31, 2018

The nature of this change meant I had to change platforms that I have no means to test. I believe I have preserved the semantics of non-linux-gnu targets, but please do apply extra scrutiny. I especially thought that freebsd looked weird, for which I left a FIXME comment.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2018
@KodrAus
Copy link
Contributor

KodrAus commented Feb 1, 2018

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned KodrAus Feb 1, 2018
@alexcrichton
Copy link
Member

@bors: r+

Looks great to me, thanks @cuviper!

@bors
Copy link
Contributor

bors commented Feb 1, 2018

📌 Commit 55b54a9 has been approved by alexcrichton

kennytm added a commit to kennytm/rust that referenced this pull request Feb 3, 2018
…ichton

Use a range to identify SIGSEGV in stack guards

Previously, the `guard::init()` and `guard::current()` functions were
returning a `usize` address representing the top of the stack guard,
respectively for the main thread and for spawned threads.  The `SIGSEGV`
handler on `unix` targets checked if a fault was within one page below that
address, if so reporting it as a stack overflow.

Now `unix` targets report a `Range<usize>` representing the guard memory,
so it can cover arbitrary guard sizes.  Non-`unix` targets which always
return `None` for guards now do so with `Option<!>`, so they don't pay any
overhead.

For `linux-gnu` in particular, the previous guard upper-bound was
`stackaddr + guardsize`, as the protected memory was *inside* the stack.
This was a glibc bug, and starting from 2.27 they are moving the guard
*past* the end of the stack.  However, there's no simple way for us to know
where the guard page actually lies, so now we declare it as the whole range
of `stackaddr ± guardsize`, and any fault therein will be called a stack
overflow.  This fixes rust-lang#47863.
bors added a commit that referenced this pull request Feb 3, 2018
Rollup of 9 pull requests

- Successful merges: #47753, #47862, #47877, #47896, #47912, #47944, #47947, #47978, #47958
- Failed merges:
kennytm added a commit to kennytm/rust that referenced this pull request Feb 4, 2018
…ichton

Use a range to identify SIGSEGV in stack guards

Previously, the `guard::init()` and `guard::current()` functions were
returning a `usize` address representing the top of the stack guard,
respectively for the main thread and for spawned threads.  The `SIGSEGV`
handler on `unix` targets checked if a fault was within one page below that
address, if so reporting it as a stack overflow.

Now `unix` targets report a `Range<usize>` representing the guard memory,
so it can cover arbitrary guard sizes.  Non-`unix` targets which always
return `None` for guards now do so with `Option<!>`, so they don't pay any
overhead.

For `linux-gnu` in particular, the previous guard upper-bound was
`stackaddr + guardsize`, as the protected memory was *inside* the stack.
This was a glibc bug, and starting from 2.27 they are moving the guard
*past* the end of the stack.  However, there's no simple way for us to know
where the guard page actually lies, so now we declare it as the whole range
of `stackaddr ± guardsize`, and any fault therein will be called a stack
overflow.  This fixes rust-lang#47863.
kennytm added a commit to kennytm/rust that referenced this pull request Feb 4, 2018
…ichton

Use a range to identify SIGSEGV in stack guards

Previously, the `guard::init()` and `guard::current()` functions were
returning a `usize` address representing the top of the stack guard,
respectively for the main thread and for spawned threads.  The `SIGSEGV`
handler on `unix` targets checked if a fault was within one page below that
address, if so reporting it as a stack overflow.

Now `unix` targets report a `Range<usize>` representing the guard memory,
so it can cover arbitrary guard sizes.  Non-`unix` targets which always
return `None` for guards now do so with `Option<!>`, so they don't pay any
overhead.

For `linux-gnu` in particular, the previous guard upper-bound was
`stackaddr + guardsize`, as the protected memory was *inside* the stack.
This was a glibc bug, and starting from 2.27 they are moving the guard
*past* the end of the stack.  However, there's no simple way for us to know
where the guard page actually lies, so now we declare it as the whole range
of `stackaddr ± guardsize`, and any fault therein will be called a stack
overflow.  This fixes rust-lang#47863.
bors added a commit that referenced this pull request Feb 4, 2018
Rollup of 10 pull requests

- Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892
- Failed merges:
bors added a commit that referenced this pull request Feb 4, 2018
Rollup of 10 pull requests

- Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892
- Failed merges:
@bors bors merged commit 55b54a9 into rust-lang:master Feb 5, 2018
@shlevy
Copy link
Contributor

shlevy commented Mar 7, 2018

Is there a recommended way to backport this to 1.24.1? We're seeing segfaults during tests when trying to bump nixpkgs to use glibc 2.27, but this patch doesn't cleanly apply. I can try to resolve manually if there's no pre-existing backport, of course.

@cuviper
Copy link
Member Author

cuviper commented Mar 7, 2018

You can borrow the patches from Fedora:
https://src.fedoraproject.org/rpms/rust/tree/1bb4d24c060915c304c9a9f86a438388e599f9c6
0001-Enable-stack-probe-tests-with-system-LLVM-5.0.patch
0002-Use-a-range-to-identify-SIGSEGV-in-stack-guards.patch

@shlevy
Copy link
Contributor

shlevy commented Mar 7, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

glibc 2.27 stack guard pages are moving
7 participants