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>
diff --git a/src/runtime/checkptr.go b/src/runtime/checkptr.go
index d429508..2d4afd5 100644
--- a/src/runtime/checkptr.go
+++ b/src/runtime/checkptr.go
@@ -7,6 +7,11 @@
import "unsafe"
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.
// Note that we allow unaligned pointers if the types they point to contain
// no pointers themselves. See issue 37298.
@@ -29,10 +34,12 @@
return false
}
- end := add(ptr, size-1)
- if uintptr(end) < uintptr(ptr) {
+ // Check that add(ptr, size-1) won't overflow. This avoids the risk
+ // of producing an illegal pointer value (assuming ptr is legal).
+ if uintptr(ptr) >= -(size - 1) {
return true
}
+ end := add(ptr, size-1)
// TODO(mdempsky): Detect when [ptr, end] contains Go allocations,
// but neither ptr nor end point into one themselves.
diff --git a/src/runtime/checkptr_test.go b/src/runtime/checkptr_test.go
index 2a5c364..d5dd101 100644
--- a/src/runtime/checkptr_test.go
+++ b/src/runtime/checkptr_test.go
@@ -26,6 +26,7 @@
}{
{"CheckPtrAlignmentPtr", "fatal error: checkptr: misaligned pointer conversion\n"},
{"CheckPtrAlignmentNoPtr", ""},
+ {"CheckPtrAlignmentNilPtr", ""},
{"CheckPtrArithmetic", "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"},
diff --git a/src/runtime/testdata/testprog/checkptr.go b/src/runtime/testdata/testprog/checkptr.go
index f76b64a..9c55613 100644
--- a/src/runtime/testdata/testprog/checkptr.go
+++ b/src/runtime/testdata/testprog/checkptr.go
@@ -4,11 +4,16 @@
package main
-import "unsafe"
+import (
+ "runtime"
+ "time"
+ "unsafe"
+)
func init() {
register("CheckPtrAlignmentNoPtr", CheckPtrAlignmentNoPtr)
register("CheckPtrAlignmentPtr", CheckPtrAlignmentPtr)
+ register("CheckPtrAlignmentNilPtr", CheckPtrAlignmentNilPtr)
register("CheckPtrArithmetic", CheckPtrArithmetic)
register("CheckPtrArithmetic2", CheckPtrArithmetic2)
register("CheckPtrSize", CheckPtrSize)
@@ -29,6 +34,35 @@
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() {
var x int
i := uintptr(unsafe.Pointer(&x))
