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 · 9 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Milestone

Comments

@mundaym
Copy link
Member

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 this to the Go1.15 milestone Mar 25, 2020
@mundaym
Copy link
Member Author

mundaym commented Mar 25, 2020

cc @laboger @cherrymui

@cherrymui
Copy link
Member

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.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 25, 2020
@mundaym
Copy link
Member Author

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
Copy link
Contributor

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.)

@odeke-em
Copy link
Member

Howdy @mundaym @randall77 @cherrymui? Anything that we can still do here for Go1.15 or shall we move this to Go1.16? Thank you.

@randall77
Copy link
Contributor

Yes, this will need to wait for 1.16.

@randall77 randall77 modified the milestones: Go1.15, Go1.16 May 31, 2020
@sahnaseredini
Copy link

sahnaseredini commented Nov 5, 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?

As far as I understood, you've already considered mitigations for Spectre v1 (Spectre-PHT), v2 (Spectre-BTB) and v5 (Spectre-RSB), right? What about Spectre v4 (Spectre-STL)? Is there any mitigation or something for it in the Go compiler?

@randall77
Copy link
Contributor

We don't have any mitigation for Spectre-STL as far as I know.

@aclements
Copy link
Member

This is certainly not going to happen for 1.16 at this point. Bumping to "Unplanned".

@aclements aclements modified the milestones: Go1.16, Unplanned Dec 1, 2020
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Projects
None yet
Development

No branches or pull requests

8 participants