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

x/sys/unix: MmapPtr upsets checkptr #70721

Open
Jille opened this issue Dec 7, 2024 · 4 comments
Open

x/sys/unix: MmapPtr upsets checkptr #70721

Jille opened this issue Dec 7, 2024 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@Jille
Copy link

Jille commented Dec 7, 2024

Go version

go version go1.23.2 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/jille/.cache/go-build'
GOENV='/home/jille/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/jille/go/pkg/mod'
GONOPROXY='src.hexon.nl'
GONOSUMDB='src.hexon.nl'
GOOS='linux'
GOPATH='/home/jille/go'
GOPRIVATE='src.hexon.nl'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go-1.23'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go-1.23/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jille/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/jille/src/ilsa/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4165437972=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I'm using unix.MmapPtr() with unix.MAP_FIXED, which triggers checkptr (for example when run with -race).

https://go.dev/play/p/3raMoMpk-YA (build with -race to trigger the error)

What did you see happen?

fatal error: checkptr: pointer arithmetic result points to invalid allocation

goroutine 1 gp=0xc0000061c0 m=0 mp=0x5d2f80 [running]:
runtime.throw({0x50bb48?, 0xc0000da000?})
	/usr/lib/go-1.23/src/runtime/panic.go:1067 +0x48 fp=0xc0000a3eb8 sp=0xc0000a3e88 pc=0x49e608
runtime.checkptrArithmetic(0xc0000da000?, {0x0, 0x0, 0x32?})
	/usr/lib/go-1.23/src/runtime/checkptr.go:69 +0x9c fp=0xc0000a3ee8 sp=0xc0000a3eb8 pc=0x43e0fc
golang.org/x/sys/unix.MmapPtr(...)
	/home/jille/go/pkg/mod/golang.org/x/[email protected]/unix/syscall_unix.go:159
main.main()
	/home/jille/src/ilsa/testmmap/testmmap.go:14 +0x125 fp=0xc0000a3f50 sp=0xc0000a3ee8 pc=0x4e0a65
runtime.main()
	/usr/lib/go-1.23/src/runtime/proc.go:272 +0x28b fp=0xc0000a3fe0 sp=0xc0000a3f50 pc=0x46ca0b
runtime.goexit({})
	/usr/lib/go-1.23/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000a3fe8 sp=0xc0000a3fe0 pc=0x4a5221

What did you expect to see?

This is the implementation of MmapPtr. The compiler can't prove this conversion from uinptr to unsafe.Pointer is safe.

func MmapPtr(fd int, offset int64, addr unsafe.Pointer, length uintptr, prot int, flags int) (ret unsafe.Pointer, err error) {
	xaddr, err := mapper.mmap(uintptr(addr), length, prot, flags, fd, offset)
	return unsafe.Pointer(xaddr), err
}
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 7, 2024
@gopherbot gopherbot added this to the Unreleased milestone Dec 7, 2024
@randall77
Copy link
Contributor

This is a correct report. You cannot remap the Go stack or heap.

You'd have to either pass nil as the addr or something previously obtained from one of the mmap functions.

@thepudds thepudds added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 7, 2024
@Jille
Copy link
Author

Jille commented Dec 10, 2024

Here is the full scope of my crime: https://pkg.go.dev/github.com/Jille/gcmmap

I agree that people shouldn't remap the Go heap or stack and expect the Go compatibility promise to apply to them.

However, if someone got here it seems unlikely they did it by accident. If they're doing something wrong with the unsafe.Pointer they're passing in, checkptr should catch it before the call. Once mmap returns I think people likely are getting what they asked for.

@randall77
Copy link
Contributor

If they're doing something wrong with the unsafe.Pointer they're passing in, checkptr should catch it before the call.

I'm not sure that's possible. It is ok by the unsafe.Pointer rules to make one with, e.g., unsafe.Pointer(&b[0]). The only thing not allowed here is to mmap it. How would we catch that before the mmap call?

Once mmap returns I think people likely are getting what they asked for.

But mmap hasn't returned. The checkptr error happens inside mmap.

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Dec 14, 2024
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. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
Status: No status
Development

No branches or pull requests

6 participants