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

Jump to bottom

x/sys/unix: Test_anyToSockaddr converts a *RawSockaddrTIPC to a pointer to a larger type #35106

Closed
tklauser opened this issue Oct 23, 2019 · 6 comments
Closed

x/sys/unix: Test_anyToSockaddr converts a *RawSockaddrTIPC to a pointer to a larger type #35106

tklauser opened this issue Oct 23, 2019 · 6 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@tklauser
Copy link
Member

https://golang.org/cl/201783 enabled -d=checkptr on the race builders. This now fails Test_anyToSockaddr:

ok  	golang.org/x/sys/cpu	1.012s
--- FAIL: Test_anyToSockaddr (0.00s)
panic: runtime error: unsafe pointer conversion [recovered]
	panic: runtime error: unsafe pointer conversion

goroutine 5 [running]:
testing.tRunner.func1(0xc0000da300)
	/workdir/go/src/testing/testing.go:881 +0x69f
panic(0x78a300, 0xc00000e240)
	/workdir/go/src/runtime/panic.go:679 +0x1b2
golang.org/x/sys/unix.Test_anyToSockaddr(0xc0000da300)
	/workdir/gopath/src/golang.org/x/sys/unix/syscall_internal_linux_test.go:33 +0x75b
testing.tRunner(0xc0000da300, 0x7bb370)
	/workdir/go/src/testing/testing.go:916 +0x19a
created by testing.(*T).Run
	/workdir/go/src/testing/testing.go:967 +0x652
FAIL	golang.org/x/sys/unix	0.015s
?   	golang.org/x/sys/windows/mkwinsyscall	[no test files]
FAIL

https://build.golang.org/log/962a4c048d92bef8dce235fb5c8e6d8f957bd977

/cc @mdempsky @bradfitz @mdlayher
@gopherbot gopherbot added this to the Unreleased milestone Oct 23, 2019
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

checkptr is correct here: the RawSockaddrAny type contains a 96-byte untyped pad, so putting a pointer to a Go-allocated unix.TIPCServiceRange hides that pointer from the garbage collector and potentially results in a use-after-free error.

@bcmills bcmills changed the title x/sys/unix: Test_anyToSockaddr fails with -d=checkptr enabled x/sys/unix: Test_anyToSockaddr encodes a Go-allocated pointer in a byte slice Oct 23, 2019
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

Hmm, maybe not. The tipcAddr method encodes its receiver to a byte array.

@mdlayher
Copy link
Member

I'll take a look at this today.

@mdlayher mdlayher self-assigned this Oct 23, 2019
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

Ah, here's the trouble. RawSockaddrAny is larger than RawSockaddrTIPC, so punning the latter to the former allows an unsafe access past the end of the object without explicit use of unsafe.

play.golang.org$ cat main.go
package main

import (
        "fmt"
        "unsafe"

        "golang.org/x/sys/unix"
)

func main() {
        fmt.Printf("sizeof(%T) = %v\n", unix.RawSockaddrTIPC{}, unsafe.Sizeof(unix.RawSockaddrTIPC{}))
        fmt.Printf("sizeof(%T) = %v\n", unix.RawSockaddrAny{}, unsafe.Sizeof(unix.RawSockaddrAny{}))
}

play.golang.org$ go run .
sizeof(unix.RawSockaddrTIPC) = 16
sizeof(unix.RawSockaddrAny) = 112

@bcmills bcmills changed the title x/sys/unix: Test_anyToSockaddr encodes a Go-allocated pointer in a byte slice x/sys/unix: Test_anyToSockaddr converts a *RawSockaddrTIPC to a pointer to a larger type Oct 23, 2019
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2019

Probably that implies that the test should copy the bytes of the RawSockaddrTIPC into a new RawSockaddrAny rather than trying to pun the pointer.

@gopherbot
Copy link

Change https://golang.org/cl/202820 mentions this issue: unix: comply with -d=checkptr in Test_anyToSockaddr

@golang golang locked and limited conversation to collaborators Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@tklauser @mdlayher @bcmills @gopherbot