Commit 627798db authored by Austin Clements's avatar Austin Clements

reflect: fix out-of-bounds pointers calling no-result method

reflect.callReflect heap-allocates a stack frame and then constructs
pointers to the arguments and result areas of that frame. However, if
there are no results, the results pointer will point past the end of
the frame allocation. If there are also no arguments, the arguments
pointer will also point past the end of the frame allocation. If the
GC observes either these pointers, it may panic.

Fix this by not constructing these pointers if these areas of the
frame are empty.

This adds a test of calling no-argument/no-result methods via reflect,
since nothing in std did this before. However, it's quite difficult to
demonstrate the actual failure because it depends on both exact
allocation patterns and on GC scanning the goroutine's stack while
inside one of the typedmemmovepartial calls.

I also audited other uses of typedmemmovepartial and
memclrNoHeapPointers in reflect, since these are the most susceptible
to this. These appear to be the only two cases that can construct
out-of-bounds arguments to these functions.

Fixes #19724.

Change-Id: I4b83c596b5625dc4ad0567b1e281bad4faef972b
Reviewed-on: https://go-review.googlesource.com/38736
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
parent 6e9ec141
...@@ -1681,6 +1681,11 @@ func (p Point) GCMethod(k int) int { ...@@ -1681,6 +1681,11 @@ func (p Point) GCMethod(k int) int {
} }
// This will be index 3. // This will be index 3.
func (p Point) NoArgs() {
// Exercise no-argument/no-result paths.
}
// This will be index 4.
func (p Point) TotalDist(points ...Point) int { func (p Point) TotalDist(points ...Point) int {
tot := 0 tot := 0
for _, q := range points { for _, q := range points {
...@@ -1709,6 +1714,15 @@ func TestMethod(t *testing.T) { ...@@ -1709,6 +1714,15 @@ func TestMethod(t *testing.T) {
t.Errorf("Type MethodByName returned %d; want 275", i) t.Errorf("Type MethodByName returned %d; want 275", i)
} }
m, ok = TypeOf(p).MethodByName("NoArgs")
if !ok {
t.Fatalf("method by name failed")
}
n := len(m.Func.Call([]Value{ValueOf(p)}))
if n != 0 {
t.Errorf("NoArgs returned %d values; want 0", n)
}
i = TypeOf(&p).Method(1).Func.Call([]Value{ValueOf(&p), ValueOf(12)})[0].Int() i = TypeOf(&p).Method(1).Func.Call([]Value{ValueOf(&p), ValueOf(12)})[0].Int()
if i != 300 { if i != 300 {
t.Errorf("Pointer Type Method returned %d; want 300", i) t.Errorf("Pointer Type Method returned %d; want 300", i)
...@@ -1723,6 +1737,15 @@ func TestMethod(t *testing.T) { ...@@ -1723,6 +1737,15 @@ func TestMethod(t *testing.T) {
t.Errorf("Pointer Type MethodByName returned %d; want 325", i) t.Errorf("Pointer Type MethodByName returned %d; want 325", i)
} }
m, ok = TypeOf(&p).MethodByName("NoArgs")
if !ok {
t.Fatalf("method by name failed")
}
n = len(m.Func.Call([]Value{ValueOf(&p)}))
if n != 0 {
t.Errorf("NoArgs returned %d values; want 0", n)
}
// Curried method of value. // Curried method of value.
tfunc := TypeOf((func(int) int)(nil)) tfunc := TypeOf((func(int) int)(nil))
v := ValueOf(p).Method(1) v := ValueOf(p).Method(1)
...@@ -1741,6 +1764,8 @@ func TestMethod(t *testing.T) { ...@@ -1741,6 +1764,8 @@ func TestMethod(t *testing.T) {
if i != 375 { if i != 375 {
t.Errorf("Value MethodByName returned %d; want 375", i) t.Errorf("Value MethodByName returned %d; want 375", i)
} }
v = ValueOf(p).MethodByName("NoArgs")
v.Call(nil)
// Curried method of pointer. // Curried method of pointer.
v = ValueOf(&p).Method(1) v = ValueOf(&p).Method(1)
...@@ -1759,6 +1784,8 @@ func TestMethod(t *testing.T) { ...@@ -1759,6 +1784,8 @@ func TestMethod(t *testing.T) {
if i != 425 { if i != 425 {
t.Errorf("Pointer Value MethodByName returned %d; want 425", i) t.Errorf("Pointer Value MethodByName returned %d; want 425", i)
} }
v = ValueOf(&p).MethodByName("NoArgs")
v.Call(nil)
// Curried method of interface value. // Curried method of interface value.
// Have to wrap interface value in a struct to get at it. // Have to wrap interface value in a struct to get at it.
...@@ -1808,6 +1835,9 @@ func TestMethodValue(t *testing.T) { ...@@ -1808,6 +1835,9 @@ func TestMethodValue(t *testing.T) {
if i != 275 { if i != 275 {
t.Errorf("Value MethodByName returned %d; want 275", i) t.Errorf("Value MethodByName returned %d; want 275", i)
} }
v = ValueOf(p).MethodByName("NoArgs")
ValueOf(v.Interface()).Call(nil)
v.Interface().(func())()
// Curried method of pointer. // Curried method of pointer.
v = ValueOf(&p).Method(1) v = ValueOf(&p).Method(1)
...@@ -1826,6 +1856,9 @@ func TestMethodValue(t *testing.T) { ...@@ -1826,6 +1856,9 @@ func TestMethodValue(t *testing.T) {
if i != 325 { if i != 325 {
t.Errorf("Pointer Value MethodByName returned %d; want 325", i) t.Errorf("Pointer Value MethodByName returned %d; want 325", i)
} }
v = ValueOf(&p).MethodByName("NoArgs")
ValueOf(v.Interface()).Call(nil)
v.Interface().(func())()
// Curried method of pointer to pointer. // Curried method of pointer to pointer.
pp := &p pp := &p
...@@ -1881,7 +1914,7 @@ func TestVariadicMethodValue(t *testing.T) { ...@@ -1881,7 +1914,7 @@ func TestVariadicMethodValue(t *testing.T) {
// Curried method of value. // Curried method of value.
tfunc := TypeOf((func(...Point) int)(nil)) tfunc := TypeOf((func(...Point) int)(nil))
v := ValueOf(p).Method(3) v := ValueOf(p).Method(4)
if tt := v.Type(); tt != tfunc { if tt := v.Type(); tt != tfunc {
t.Errorf("Variadic Method Type is %s; want %s", tt, tfunc) t.Errorf("Variadic Method Type is %s; want %s", tt, tfunc)
} }
......
...@@ -630,8 +630,11 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) { ...@@ -630,8 +630,11 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
args := framePool.Get().(unsafe.Pointer) args := framePool.Get().(unsafe.Pointer)
// Copy in receiver and rest of args. // Copy in receiver and rest of args.
// Avoid constructing out-of-bounds pointers if there are no args.
storeRcvr(rcvr, args) storeRcvr(rcvr, args)
if argSize-ptrSize > 0 {
typedmemmovepartial(frametype, unsafe.Pointer(uintptr(args)+ptrSize), frame, ptrSize, argSize-ptrSize) typedmemmovepartial(frametype, unsafe.Pointer(uintptr(args)+ptrSize), frame, ptrSize, argSize-ptrSize)
}
// Call. // Call.
call(frametype, fn, args, uint32(frametype.size), uint32(retOffset)) call(frametype, fn, args, uint32(frametype.size), uint32(retOffset))
...@@ -641,6 +644,8 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) { ...@@ -641,6 +644,8 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
// a receiver) is different from the layout of the fn call, which has // a receiver) is different from the layout of the fn call, which has
// a receiver. // a receiver.
// Ignore any changes to args and just copy return values. // Ignore any changes to args and just copy return values.
// Avoid constructing out-of-bounds pointers if there are no return values.
if frametype.size-retOffset > 0 {
callerRetOffset := retOffset - ptrSize callerRetOffset := retOffset - ptrSize
if runtime.GOARCH == "amd64p32" { if runtime.GOARCH == "amd64p32" {
callerRetOffset = align(argSize-ptrSize, 8) callerRetOffset = align(argSize-ptrSize, 8)
...@@ -650,6 +655,7 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) { ...@@ -650,6 +655,7 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
unsafe.Pointer(uintptr(args)+retOffset), unsafe.Pointer(uintptr(args)+retOffset),
retOffset, retOffset,
frametype.size-retOffset) frametype.size-retOffset)
}
// This is untyped because the frame is really a stack, even // This is untyped because the frame is really a stack, even
// though it's a heap object. // though it's a heap object.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment