From: Ian Lance Taylor <iant@golang.org>
Date: Wed, 20 Aug 2025 00:00:29 +0000 (-0700)
Subject: runtime: panic on AddCleanup with self pointer
X-Git-Tag: go1.26rc1~99
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=06412288cf;p=gostls13.git

runtime: panic on AddCleanup with self pointer

For #75066

Change-Id: Ifd555586fb448e7510fed16372648bdd7ec0ab4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/697535
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
---

diff --git a/src/runtime/mcleanup.go b/src/runtime/mcleanup.go
index fc71af9f3f..d190653213 100644
--- a/src/runtime/mcleanup.go
+++ b/src/runtime/mcleanup.go
@@ -84,7 +84,8 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
 
 	// Check that arg is not equal to ptr.
 	argType := abi.TypeOf(arg)
-	if kind := argType.Kind(); kind == abi.Pointer || kind == abi.UnsafePointer {
+	kind := argType.Kind()
+	if kind == abi.Pointer || kind == abi.UnsafePointer {
 		if unsafe.Pointer(ptr) == *((*unsafe.Pointer)(unsafe.Pointer(&arg))) {
 			panic("runtime.AddCleanup: ptr is equal to arg, cleanup will never run")
 		}
@@ -119,7 +120,7 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
 	*argv = arg
 
 	// Find the containing object.
-	base, _, _ := findObject(usptr, 0, 0)
+	base, span, _ := findObject(usptr, 0, 0)
 	if base == 0 {
 		if isGoPointerWithoutSpan(unsafe.Pointer(ptr)) {
 			// Cleanup is a noop.
@@ -128,6 +129,22 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
 		panic("runtime.AddCleanup: ptr not in allocated block")
 	}
 
+	// Check that arg is not within ptr.
+	if kind == abi.Pointer || kind == abi.UnsafePointer {
+		argPtr := uintptr(*(*unsafe.Pointer)(unsafe.Pointer(&arg)))
+		if argPtr >= base && argPtr < base+span.elemsize {
+			// It's possible that both pointers are separate
+			// parts of a tiny allocation, which is OK.
+			// We side-stepped the tiny allocator above for
+			// the allocation for the cleanup,
+			// but the argument itself can still overlap
+			// with the value to which the cleanup is attached.
+			if span.spanclass != tinySpanClass {
+				panic("runtime.AddCleanup: ptr is within arg, cleanup will never run")
+			}
+		}
+	}
+
 	// Create another G if necessary.
 	if gcCleanups.needG() {
 		gcCleanups.createGs()
diff --git a/src/runtime/mcleanup_test.go b/src/runtime/mcleanup_test.go
index 341d30afa7..28fc7f407f 100644
--- a/src/runtime/mcleanup_test.go
+++ b/src/runtime/mcleanup_test.go
@@ -337,6 +337,23 @@ func TestCleanupLost(t *testing.T) {
 	}
 }
 
+func TestCleanupUnreachable(t *testing.T) {
+	defer func() {
+		if recover() == nil {
+			t.Error("AddCleanup failed to detect self-pointer")
+		}
+	}()
+
+	type T struct {
+		p *byte // use *byte to avoid tiny allocator
+		f int
+	}
+	v := &T{}
+	runtime.AddCleanup(v, func(*int) {
+		t.Error("cleanup ran unexpectedly")
+	}, &v.f)
+}
+
 // BenchmarkAddCleanupAndStop benchmarks adding and removing a cleanup
 // from the same allocation.
 //