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.

Sign up for GitHub

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

Jump to bottom

runtime: canonical 48-bit address bug in lfstack #49405

Open
Sonicadvance1 opened this issue Nov 6, 2021 · 2 comments
Open

runtime: canonical 48-bit address bug in lfstack #49405

Sonicadvance1 opened this issue Nov 6, 2021 · 2 comments
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.
Milestone
Unplanned

Comments

@Sonicadvance1
Copy link

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

go version go1.16.2 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="/home/ryanh/.cache/go-build"
GOENV="/home/ryanh/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ryanh/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ryanh/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.16"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.16/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.2"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1471403295=/tmp/go-build -gno-record-gcc-switches"

What did you do?

If lfstack gets any pointer on amd64 platforms where the top bit is set then an assert is thrown.
In most cases this won't occur since the kernel will consume the top bit of VA, leaving the userspace with 47bits of VA.
This is a logic bug around address canonicalization.

Check is here: https://github.com/golang/go/blob/master/src/runtime/lfstack.go#L28
Pointer sign extension is here: https://github.com/golang/go/blob/master/src/runtime/lfstack_64bit.go#L52

I don't have a basic application or environment for reproducing this. Easy to see the logic bug.

What did you expect to see?

While it is correct that you must canonicalize the pointer by sign extending it on 48bit systems (I'm not even going to bring up LA57). The check inside of lfstack is incorrect since it assumes the sign extension will never occur. Thus assuming it will never receive a true 48-bit pointer.
This assumption is broken in environments where you get 48-bit pointers with the high bit set.
This is purely a simple logic bug, it shouldn't assert in this case.

What did you see instead?

An assert on 48-bit address with high bit set.
@Sonicadvance1 Sonicadvance1 mentioned this issue Nov 6, 2021
Closed
Sonicadvance1 added a commit to Sonicadvance1/FEX that referenced this issue Nov 6, 2021
@Sonicadvance1
Allocator: Reserve upper 128TB of VA on 64-bit process
df2f1ad 
Only a partial fix for FEX-Emu#1330, still needs preemption disabled to work.

On x86-64 hosts the Linux kernel resides in the top bit of VA which
isn't mapped in to userspace.
This means that userspace will never receive pointers living with that
top bit set unless you're running a 57bit VA host.

This results in userspace pointers never needing to do the sign
extending pointer canonicalization. But additionally some applications
actually don't understand the pointer canonicalization.
This results in bugs like: golang/go#49405
Now if you're running on a 57bit VA host, this will end up behaving like
FEX but it seems like no one in golang land has really messed with 57bit
VA yet.

In AArch64, when configured with a 48bit VA, the userspace gets the full
48bit VA space and on EL mode switch has the full address range change
to the kernel's 48bit VA.
This means that we will /very/ likely allocate pointers in the high
48bit space since Linux currently allocates top-down.

So behave more like x86-64, hide the top 128TB of memory space from the
guest before boot.

Testing: Took the M1Max 15ms to 21ms allocate the top 128TB.
@Sonicadvance1 Sonicadvance1 mentioned this issue Nov 6, 2021
Merged
@ianlancetaylor ianlancetaylor changed the title AMD64: Canonical 48-bit address bug in lfstack runtime: canonical 48-bit address bug in lfstack Nov 7, 2021
@ianlancetaylor
Copy link
Contributor

Thanks. Note that this code is deeply system dependent. Can the problem you mention actually happen on any existing system? If not, it's not worth worrying about.

@Sonicadvance1
Copy link
Author

Currently the only real concern is if anyone tries running on Intel's latest server platforms with LA57 enabled.
Which doesn't pass the userspace a 48-bit (or higher) pointer unless explicitly asked.
Otherwise it would likely just be toy environments attempting a 48-bit VA space like AArch64 platforms. Switching memory mappings on CPL change.

@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 8, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@Sonicadvance1 @ianlancetaylor @gopherbot @seankhliao @thanm