runtime: don't crash on nil pointers in checkptrAlignment

Ironically, checkptrAlignment had a latent case of bad pointer
arithmetic: if ptr is nil, then `add(ptr, size-1)` might produce an
illegal pointer value.

The fix is to simply check for nil at the top of checkptrAlignment,
and short-circuit if so.

This CL also adds a more explicit bounds check in checkptrStraddles,
rather than relying on `add(ptr, size-1)` to wrap around. I don't
think this is necessary today, but it seems prudent to be careful.

Fixes #47430.

Change-Id: I5c50b2f7f41415dbebbd803e1b8e7766ca95e1fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/338029
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This commit is contained in:
Matthew Dempsky 2021-07-27 18:30:38 -07:00
parent 7cd10c1149
commit b39e0f461c
3 changed files with 45 additions and 3 deletions

View file

@ -7,6 +7,11 @@ package runtime
import "unsafe" import "unsafe"
func checkptrAlignment(p unsafe.Pointer, elem *_type, n uintptr) { func checkptrAlignment(p unsafe.Pointer, elem *_type, n uintptr) {
// nil pointer is always suitably aligned (#47430).
if p == nil {
return
}
// Check that (*[n]elem)(p) is appropriately aligned. // Check that (*[n]elem)(p) is appropriately aligned.
// Note that we allow unaligned pointers if the types they point to contain // Note that we allow unaligned pointers if the types they point to contain
// no pointers themselves. See issue 37298. // no pointers themselves. See issue 37298.
@ -29,10 +34,12 @@ func checkptrStraddles(ptr unsafe.Pointer, size uintptr) bool {
return false return false
} }
end := add(ptr, size-1) // Check that add(ptr, size-1) won't overflow. This avoids the risk
if uintptr(end) < uintptr(ptr) { // of producing an illegal pointer value (assuming ptr is legal).
if uintptr(ptr) >= -(size - 1) {
return true return true
} }
end := add(ptr, size-1)
// TODO(mdempsky): Detect when [ptr, end] contains Go allocations, // TODO(mdempsky): Detect when [ptr, end] contains Go allocations,
// but neither ptr nor end point into one themselves. // but neither ptr nor end point into one themselves.

View file

@ -26,6 +26,7 @@ func TestCheckPtr(t *testing.T) {
}{ }{
{"CheckPtrAlignmentPtr", "fatal error: checkptr: misaligned pointer conversion\n"}, {"CheckPtrAlignmentPtr", "fatal error: checkptr: misaligned pointer conversion\n"},
{"CheckPtrAlignmentNoPtr", ""}, {"CheckPtrAlignmentNoPtr", ""},
{"CheckPtrAlignmentNilPtr", ""},
{"CheckPtrArithmetic", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"}, {"CheckPtrArithmetic", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"},
{"CheckPtrArithmetic2", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"}, {"CheckPtrArithmetic2", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"},
{"CheckPtrSize", "fatal error: checkptr: converted pointer straddles multiple allocations\n"}, {"CheckPtrSize", "fatal error: checkptr: converted pointer straddles multiple allocations\n"},

View file

@ -4,11 +4,16 @@
package main package main
import "unsafe" import (
"runtime"
"time"
"unsafe"
)
func init() { func init() {
register("CheckPtrAlignmentNoPtr", CheckPtrAlignmentNoPtr) register("CheckPtrAlignmentNoPtr", CheckPtrAlignmentNoPtr)
register("CheckPtrAlignmentPtr", CheckPtrAlignmentPtr) register("CheckPtrAlignmentPtr", CheckPtrAlignmentPtr)
register("CheckPtrAlignmentNilPtr", CheckPtrAlignmentNilPtr)
register("CheckPtrArithmetic", CheckPtrArithmetic) register("CheckPtrArithmetic", CheckPtrArithmetic)
register("CheckPtrArithmetic2", CheckPtrArithmetic2) register("CheckPtrArithmetic2", CheckPtrArithmetic2)
register("CheckPtrSize", CheckPtrSize) register("CheckPtrSize", CheckPtrSize)
@ -29,6 +34,35 @@ func CheckPtrAlignmentPtr() {
sink2 = (**int64)(unsafe.Pointer(uintptr(p) + 1)) sink2 = (**int64)(unsafe.Pointer(uintptr(p) + 1))
} }
// CheckPtrAlignmentNilPtr tests that checkptrAlignment doesn't crash
// on nil pointers (#47430).
func CheckPtrAlignmentNilPtr() {
var do func(int)
do = func(n int) {
// Inflate the stack so runtime.shrinkstack gets called during GC
if n > 0 {
do(n - 1)
}
var p unsafe.Pointer
_ = (*int)(p)
}
go func() {
for {
runtime.GC()
}
}()
go func() {
for i := 0; ; i++ {
do(i % 1024)
}
}()
time.Sleep(time.Second)
}
func CheckPtrArithmetic() { func CheckPtrArithmetic() {
var x int var x int
i := uintptr(unsafe.Pointer(&x)) i := uintptr(unsafe.Pointer(&x))