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

Please add retpoline support. #54637

Closed
dancrossnyc opened this issue Sep 28, 2018 · 7 comments
Closed

Please add retpoline support. #54637

dancrossnyc opened this issue Sep 28, 2018 · 7 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR.

Comments

@dancrossnyc
Copy link

For a project at Google, we need retpoline support. Taylor Cramer suggested I open an issue; my initial search didn't reveal any pre-existing issues that seemed particularly relevant. There was a reddit thread that stalled after identifying some blocking issues (LLD integration, LLVM upgrades, etc) all of which have, I believe, subsequently been addressed. https://www.reddit.com/r/rust/comments/7saomi/status_of_spectre_variant_2_mitigation_in_rust/

As I understand it, the heavy lifting for this is in LLVM and it simply needs to be plumbed through.

Put another way, just as clang provides -mretpoline (https://reviews.llvm.org/D41723), I'm looking for something similar in the Rust toolchain.

Thanks!

@csmoe csmoe added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Sep 28, 2018
@alexcrichton
Copy link
Member

Some simple testing in clang shows that -C target-feature=+retpoline may be sufficient in Rust to work here. Does that work for you?

@eddyb
Copy link
Member

eddyb commented Sep 28, 2018

cc @rust-lang/compiler

@GabrielMajeri
Copy link
Contributor

GabrielMajeri commented Sep 29, 2018

I'm no security expert, but from what I can tell from the LLVM code, this specific retpoline patch changes the PLT?

Because it just so happens we have a PR which will disable the PLT by default and it seems that it might reduce the performance impact of retpoline (and reduce the attack surface). If you also build your C/C++ libraries with a global CFLAGS=-fno-plt you should be able to 100% remove the PLT from your binaries.

@nagisa
Copy link
Member

nagisa commented Sep 29, 2018

We should still support retpolines when @plt is used. That being said, it seems to me that this is almost entirely a linker’s job (i.e. all of the support is being added to lld and I haven’t seen any patches against LLVM itself). This means that implementing this is probably as easy as passing the relevant flags to linker and can be done even now (-Clink-args) without any explicit support from the compiler?

EDIT: OTOH from what @alexcrichton said above, it probably does need compiler support, but still no explicit changes to rustc are necessary.

@dancrossnyc
Copy link
Author

Thanks all! I'll take what's written here and see if I can plumb the appropriate flags through cargo etc to get retpolines in the generated binary. I'll report back with results.

@cyplo
Copy link
Contributor

cyplo commented Dec 23, 2018

Heya, any news on this one @dancrossnyc ?

@dancrossnyc
Copy link
Author

Sorry for the delayed response; I was traveling over the new year.

We put this in a while back and it seems to be working as expected. Thanks, all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

No branches or pull requests

7 participants