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: spectre options don't apply to dependent packages #40866

Closed
prattmic opened this issue Aug 18, 2020 · 2 comments
Closed

cmd/compile: spectre options don't apply to dependent packages #40866

prattmic opened this issue Aug 18, 2020 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@prattmic
Copy link
Member

Maybe I am holding things wrong, but it seems like the spectre mitigations are only applying to the main package, not the entire program? (Perhaps this only applies to retpoline, I haven't checked nospec indexing yet).

Below is a minimal repro. I initially discovered this when building a large application and finding repoline calls only in package main, where it is implausible that nothing else in the program would be eligible.

What version of Go are you using (go version)?

$ go version
go version devel +5c7748dc9d linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/mpratt/.cache/go-build"
GOENV="/usr/local/google/home/mpratt/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/usr/local/google/home/mpratt/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/usr/local/google/home/mpratt/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/google-golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/usr/local/google/home/mpratt/Downloads/spectre/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build876651712=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Create a basic package structure: https://play.golang.org/p/QE_wQTBkQZ7

Build with spectre mitigations: go build -gcflags=-spectre=all -asmflags=-spectre=all

What did you expect to see?

Retpoline calls on the calls to fooer.Foo() in main and barrer.Bar() in bar.CallBarrer().

What did you see instead?

Retpoline calls only in package main.

TEXT main.main(SB) /usr/local/google/home/mpratt/Downloads/spectre/main.go
...
  main.go:22            0x45f800                e89bffffff              CALL main.newFoo(SB)
  main.go:22            0x45f805                488b0424                MOVQ 0(SP), AX
  main.go:22            0x45f809                488b4c2408              MOVQ 0x8(SP), CX
  main.go:23            0x45f80e                488b4018                MOVQ 0x18(AX), AX
  main.go:23            0x45f812                48890c24                MOVQ CX, 0(SP)
  main.go:23            0x45f816                e8c5cdffff              CALL runtime.retpolineAX(SB)
...
TEXT example.com/spectre/bar.CallBarrer(SB) /usr/local/google/home/mpratt/Downloads/spectre/bar/bar.go
...
  bar.go:19             0x45f760                e89bffffff              CALL example.com/spectre/bar.newBar(SB)
  bar.go:19             0x45f765                488b0424                MOVQ 0(SP), AX
  bar.go:19             0x45f769                488b4c2408              MOVQ 0x8(SP), CX
  bar.go:20             0x45f76e                488b4018                MOVQ 0x18(AX), AX
  bar.go:20             0x45f772                48890c24                MOVQ CX, 0(SP)
  bar.go:20             0x45f776                ffd0                    CALL AX

cc @rsc

@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 18, 2020
@prattmic prattmic added this to the Go1.16 milestone Aug 18, 2020
@prattmic
Copy link
Member Author

This seems to apply to the index mitigation as well:

https://play.golang.org/p/PFa557gxe7I

TEXT main.indexSlice(SB) /usr/local/google/home/mpratt/Downloads/spectre/main.go
...
  main.go:28            0x45f92e                488b4c2428              MOVQ 0x28(SP), CX
  main.go:28            0x45f933                4883f901                CMPQ $0x1, CX
  main.go:28            0x45f937                7626                    JBE 0x45f95f
  main.go:28            0x45f939                b801000000              MOVL $0x1, AX
  main.go:28            0x45f93e                b900000000              MOVL $0x0, CX
  main.go:28            0x45f943                480f46c1                CMOVBE CX, AX
  main.go:28            0x45f947                488b4c2420              MOVQ 0x20(SP), CX
  main.go:28            0x45f94c                488b04c1                MOVQ 0(CX)(AX*8), AX
TEXT example.com/spectre/bar.indexSlice(SB) /usr/local/google/home/mpratt/Downloads/spectre/bar/bar.go
...
  bar.go:24             0x45f7ce                488b4c2428              MOVQ 0x28(SP), CX
  bar.go:24             0x45f7d3                4883f901                CMPQ $0x1, CX                    
  bar.go:24             0x45f7d7                7618                    JBE 0x45f7f1
  bar.go:24             0x45f7d9                488b442420              MOVQ 0x20(SP), AX
  bar.go:24             0x45f7de                488b4008                MOVQ 0x8(AX), AX

@prattmic
Copy link
Member Author

Thanks @cherrymui for pointing out that the invocation should be go build -gcflags=all=-spectre=all -asmflags=all=-spectre=all to apply to all packages. It works correctly with that.

I'll update https://github.com/golang/go/wiki/Spectre#example to be more clear.

@golang golang locked and limited conversation to collaborators Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants