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 d42950844b58f93f61eec33fa80c7858ddcb3dbe..2d4afd5cf6838227deeb8845655b72b45a6353d5 100644 (file)
--- a/src/runtime/checkptr.go
+++ b/src/runtime/checkptr.go
@@ -7,6 +7,11 @@ package runtime
 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 @@ func checkptrStraddles(ptr unsafe.Pointer, size uintptr) bool {
                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 2a5c364e97bd0fc0dd106dac184181ed18b8cb71..d5dd101adbe051164a9f23f858dfd46d99f29c66 100644 (file)
--- a/src/runtime/checkptr_test.go
+++ b/src/runtime/checkptr_test.go
@@ -26,6 +26,7 @@ func TestCheckPtr(t *testing.T) {
        }{
                {"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 f76b64ad96ff68833730658c0dede09be0bcb61a..9c5561396e5d43b335c3b270201025809c307f1c 100644 (file)
--- 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 @@ func CheckPtrAlignmentPtr() {
        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))