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

Disable backtraces on OSX #45760

Closed

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Nov 4, 2017

This PR is a revert of #44251 which disables backtraces via RUST_BACKTRACE=1 on OSX. It was discovered in #45731 that OSX binaries generating a backtrace started generating a segfault in the range of 4279e2b...8493813. The most likely candidate PR in that range is #45400 which switched to compiling libtest with ThinLTO. Furthermore it's probably mostly likely that libbacktrace is segfaulting due to #45511, or otherwise bad DWARF data in libtest.

Now the bad DWARF information is certainly a bug with ThinLTO that needs to be fixed! Despite this, however, this is a presumed resurgence of #21889 where you can feed arbitrary data to libbacktrace effectively, and if it segfaults that's presumably a security vulnerability. In that sense a fix to DWARF + ThinLTO is not enough, but this indicates a deeper underlying problem that OSX binaries may be vulnerable to the situation described in #21889.

As a result, this PR reverts backtrace support for OSX, which can return once we're confident enough in the implementation of libbacktrace for OSX. I'm also going to nominate this for a beta backport as it's something we'll want to get out pretty soon.

Closes #45731

@alexcrichton alexcrichton added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 4, 2017
@alexcrichton
Copy link
Member Author

cc @rust-lang/libs
cc @JohnColanduoni

@rust-highfive
Copy link
Collaborator

r? @sfackler

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

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 4, 2017
@kennytm
Copy link
Member

kennytm commented Nov 4, 2017

cc #24346

@sfackler
Copy link
Member

sfackler commented Nov 4, 2017

Is there a way of doing this that's less invasive to the libbacktrace source?

@JohnColanduoni
Copy link
Contributor

JohnColanduoni commented Nov 4, 2017

It's worth noting that if the vulnerability is indeed in the DWARF parser it is likely on other platforms as well; there is no OS X specific code in the DWARF parser and its interactions with it are minimal (it just searches for the appropriate named sections).

The stack trace from #45731 makes me doubt that this is the case unless the given crash is due to the DWARF parser trashing memory and the Mach-O code simply happens to be the one to step in it.

@sfackler The only this that needs to reverted to prevent the code from running is the changes to the cfg attributes in sys_common/mod.rs and sys/unix/backtrace/printing/mod.rs from the first commit. The Mach-O code will not be compiled into libbacktrace on non-Apple platforms.

@JohnColanduoni
Copy link
Contributor

I tracked down the crash in #45731. It turns out the SIGBUS was due to an IO error (attempt to access page in empty mmaped file), not an errant memory access.

@alexcrichton
Copy link
Member Author

Thanks for the investigation @JohnColanduoni! Do you have a fix available locally for this? If so I think we can probably just apply that and avoid reverting.

@sfackler probably, I just did the easiest thing of reverting the PR rather than trying to pick-and-choose what happened.

@sfackler
Copy link
Member

sfackler commented Nov 6, 2017

Ah gotcha, I didn't realize the OSX stuff was already a patch on top of libbacktrace.

@JohnColanduoni
Copy link
Contributor

@alexcrichton This should do it: #45866.

@alexcrichton
Copy link
Member Author

Awesome! Closing in favor of that

@alexcrichton alexcrichton deleted the disable-osx-backtrace branch November 8, 2017 14:41
@nikomatsakis nikomatsakis removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 16, 2017
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.

cargo test with RUST_BACKTRACE causes SIGBUS
6 participants