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

cmd/compile: consider extending '-spectre' option to other architectures #38066

Open
mundaym opened this issue Mar 25, 2020 · 4 comments
Open

cmd/compile: consider extending '-spectre' option to other architectures #38066

mundaym opened this issue Mar 25, 2020 · 4 comments

Comments

@mundaym
Copy link
Member

@mundaym mundaym commented Mar 25, 2020

@rsc recently added a -spectre flag to the compiler and implemented a couple of speculative execution exploit mitigations on amd64. These involved using conditional moves to prevent out of bounds indexed memory accesses (-spectre=index added in https://golang.org/cl/222660) and making use of call stack optimizations to prevent indirect function calls being speculatively executed (-spectre=ret added in https://golang.org/cl/222661).

My understanding is that these mitigations could be important when the Go program being compiled is something like a hypervisor or operating system that has access to data or functionality that guest programs should be restricted from accessing. I'm still in the process of getting my head round these changes and how they might apply to other CPU architectures but my understanding so far is that, to be totally safe, we should apply similar mitigations on other platforms unless there are hardware mitigations in place.

The new flag also has potential for cross-platform incompatibilities since currently, for example, -spectre=index will cause the compiler to fail on non-amd64 platforms. It would be nice if these flags 'just worked' on other platforms.

I personally am responsible for maintaining the s390x port. I think it would be straightforward to implement equivalent mitigations on s390x:

  1. index: use conditional moves in exactly the same way as the amd64 backend
  2. ret: use EX{,RL} (execute) instructions to indirectly call branch instructions (more info here:
    https://www.phoronix.com/scan.php?page=news_item&px=S390-Expoline-Linux-4.16)

Does anyone have any thoughts on what we should do for other architectures, if anything?

@mundaym mundaym added the Security label Mar 25, 2020
@mundaym mundaym added this to the Go1.15 milestone Mar 25, 2020
@mundaym

This comment has been minimized.

Copy link
Member Author

@mundaym mundaym commented Mar 25, 2020

@cherrymui

This comment has been minimized.

Copy link
Contributor

@cherrymui cherrymui commented Mar 25, 2020

Yeah, this is probably a good thing. Thanks @mundaym

As this is highly related to the microarchitecture detail of the CPUs, it is probably good to demonstrate the problem as a first step.

And I'd imagine the implementation detail will differ across architectures. For example, retpoline is highly coupled with how RET instruction is speculated on x86. On LR machines things will probably be different? I think it would be good to know the mitigations we plan to do for a given architecture. Are there a list of mitigations for the architecture, say, from a C compiler or other language project? Thanks.

@mundaym

This comment has been minimized.

Copy link
Member Author

@mundaym mundaym commented Mar 25, 2020

As this is highly related to the microarchitecture detail of the CPUs, it is probably good to demonstrate the problem as a first step.

Yeah, I have no idea how to go about testing this... Maybe that will come out of the amd64 work? I doubt any spectre tests will work reliably on the buildbots though!

On LR machines things will probably be different?

Yeah, s390x can use expolines, but I don't know about other LR architectures.

Are there a list of mitigations for the architecture, say, from a C compiler or other language project?

I'll see what I can find, I haven't been following them that closely and have only just started looking into this today. If someone else has such a list handy for any architectures then please post it :)

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Mar 25, 2020

There is a test in here: https://go-review.googlesource.com/c/go/+/222978
It will need some adapting for s390x, but should be doable.
(That CL was reverted, but should go in again at some point.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.