go: cmd/compile: incorrect assignment to uint64 via pointer converted to *uint16 (new in 1.7)

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

go version go1.7 darwin/amd64 and go version go1.7 linux/amd64.

This issue is not present in go1.6.2.

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

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

and

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

Assign a uint16 (or uint32) to a previously unused uint64 via converted pointer on a little-endian architecture. go run the following program:

package main

import (
    "fmt"
    "unsafe"
)

func main() {
    var x uint64
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)

    var y uint64
    fmt.Printf("%x\n", y)
    *(*uint16)(unsafe.Pointer(&y)) = 0xffee
    fmt.Printf("%x\n", y)
}

What did you expect to see?

ffee
0
ffee

What did you see instead?

c42000ffee
0
ffee

A test that passes with Go 1.6 but not with Go 1.7:

// +build amd64

package main

import (
    "fmt"
    "unsafe"
)

func ExampleUint16NoAccessBeforeAssignment() {
    var x uint64
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)
    // Output:
    // ffee
}

func ExampleUint16AccessBeforeAssignment() {
    var x uint64
    fmt.Printf("%x\n", x)
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)
    // Output:
    // 0
    // ffee
}

func ExampleUint32NoAccessBeforeAssignment() {
    var x uint64
    *(*uint32)(unsafe.Pointer(&x)) = 0xffeeddcc
    fmt.Printf("%x\n", x)
    // Output:
    // ffeeddcc
}

func ExampleUint32AccessBeforeAssignment() {
    var x uint64
    fmt.Printf("%x\n", x)
    *(*uint32)(unsafe.Pointer(&x)) = 0xffeeddcc
    fmt.Printf("%x\n", x)
    // Output:
    // 0
    // ffeeddcc
}

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 23 (20 by maintainers)

Most upvoted comments

We should fix this. The failure mode is bad - I believe we’re exposing uninitialized stack memory this way. That’s a (very minor, but still) security bug.

randall77 on Aug 17, 2016

Keith, how can use of unsafe not be a security violation in general?

Package unsafe documents several patterns that users are allowed to assume will remain safe, one of which includes (at least by my best interpretation) converting *uint64 to *uint16. If there were no safe uses of package unsafe, then there wouldn’t be any point in providing it.

What prevents taking address of auto, casting to *byte[100000], and happily iterating over it?

That’s not one of the patterns that are documented to be safe. Conversion of *T1 to *T2 is only safe if Sizeof(T2) <= Sizeof(T1), which won’t be true in the scenario you’re asking about (since we don’t allocate 100,000+ byte objects on the stack).

and it was already broken,

Do you mean that the pre-SSA compiler didn’t support conversions from *uint64 to *uint16 either? Or you think the user code was broken for relying on patterns that you don’t agree should be valid?

mdempsky on Aug 18, 2016

https://golang.org/pkg/unsafe/#Pointer says:

(1) Conversion of a *T1 to Pointer to *T2.

Provided that T2 is no larger than T1 and that the two share an equivalent memory layout, this conversion allows reinterpreting data of one type as data of another type.

My interpretation of “equivalent memory layout” is referring to having the same pointer slots. Since uint16 is no larger than uint64 and neither contain pointers, it seems explicitly documented to be valid.

If not, we should clarify the documentation to define “memory layout”. That term isn’t used in the Go language spec or memory model.

mdempsky on Aug 17, 2016

I think that we should not be bound by any particular artifacts of the previous compiler’s treatment of unsafe code, because the full set of those artifacts is not specified. I also think it was a mistake to “document” this behavior for end-user use, because people will write unsafe code that happens to pass tests with a particular Go release and believe that therefore they got it right. We don’t have a test suite for what unsafe behavior we support except for the runtime itself, and compiler writers aren’t good at imaging the crazy things that people might do with unsafe code. And lacking such a test suite, for unsafe, I think it’s fine for compiler-writers to push back hard on “bugs” involving unsafe code.

Unsafe code is generally a security violation, because no matter what we say in documentation, we don’t check it mechanically. It’s pretty much the whole point of actual security violations that they do an end-run on documented restrictions. Or as I asked, “what prevents?”

dr2chase on Aug 18, 2016