Don't use runtime.Pinner to avoid false positive in cgo pointer check (#274)

* don't use runtime.Pinner to avoid false positive in cgo pointer check

* fix TestIssue71943

* always return a referenceable pointer

* use pbaseNeverEmpty

* simplify code

* fix hashOneShot

* fix evpHashSign

Author: qmuntal

cmd/mkcgo/generate.go

@@ -407,13 +407,20 @@ func generateCFn(typedefs map[string]string, fn *mkcgo.Func, w io.Writer) {
 }
 
 // paramToGo converts C parameter p to Go parameter.
-func paramToGo(p *mkcgo.Param) string {
+func paramToGo(p *mkcgo.Param, hidden bool) string {
 	goType, needCast := cTypeToGo(p.Type, true)
 	if !needCast {
+		if hidden && goType == "unsafe.Pointer" {
+			// The parameter is not annotated with cgoCheckPointer by cgo
+			// if it has the form unsafe.Pointer(&...).
+			// See https://go-review.googlesource.com/c/go/+/404295.
+			// TODO: support other types.
+			return fmt.Sprintf("unsafe.Pointer(&*(*byte)(%s))", p.Name)
+		}
 		return p.Name
 	}
 	switch {
-	case goType == "unsafe.Pointer" || goType == "":
+	case goType == "":
 		return p.Name
 	case goType[0] == '*':
 		return fmt.Sprintf("(%s)(unsafe.Pointer(%s))", goType, p.Name)
@@ -540,7 +547,7 @@ func fnToGoParams(fn *mkcgo.Func) string {
 // argList returns source code for C parameters for function f.
 func fnToGoArgs(fn *mkcgo.Func) string {
 	return join(fn.Params, func(_ int, p *mkcgo.Param) string {
-		return paramToGo(p)
+		return paramToGo(p, slices.Contains(fn.NoCheckPtrParams, p.Name))
 	}, ", ")
 }
 

evp.go

@@ -441,8 +441,10 @@ func evpHashSign(withKey withKeyFunc, h crypto.Hash, msg []byte) ([]byte, error)
 	}); err != nil {
 		return nil, err
 	}
-	if _, err := ossl.EVP_DigestUpdate(ctx, unsafe.Pointer(base(msg)), len(msg)); err != nil {
-		return nil, err
+	if len(msg) > 0 {
+		if _, err := ossl.EVP_DigestUpdate(ctx, pbase(msg), len(msg)); err != nil {
+			return nil, err
+		}
 	}
 	// Obtain the signature length
 	if _, err := ossl.EVP_DigestSignFinal(ctx, nil, &outLen); err != nil {
@@ -472,8 +474,10 @@ func evpHashVerify(withKey withKeyFunc, h crypto.Hash, msg, sig []byte) error {
 	}); err != nil {
 		return err
 	}
-	if _, err := ossl.EVP_DigestUpdate(ctx, unsafe.Pointer(base(msg)), len(msg)); err != nil {
-		return err
+	if len(msg) > 0 {
+		if _, err := ossl.EVP_DigestUpdate(ctx, pbase(msg), len(msg)); err != nil {
+			return err
+		}
 	}
 	if _, err := ossl.EVP_DigestVerifyFinal(ctx, base(sig), len(sig)); err != nil {
 		return err

hash.go

@@ -19,12 +19,7 @@ import (
 const maxHashSize = 64
 
 func hashOneShot(ch crypto.Hash, p []byte, sum []byte) bool {
-	if len(p) > 0 {
-		var pinner runtime.Pinner
-		defer pinner.Unpin()
-		pinner.Pin(&p[0])
-	}
-	_, err := ossl.EVP_Digest(pbase(p), len(p), base(sum), nil, loadHash(ch).md, nil)
+	_, err := ossl.EVP_Digest(pbaseNeverEmpty(p), len(p), base(sum), nil, loadHash(ch).md, nil)
 	return err == nil
 }
 
@@ -253,8 +248,7 @@ type evpHash struct {
 	// ctx2 is used in evpHash.sum to avoid changing
 	// the state of ctx. Having it here allows reusing the
 	// same allocated object multiple times.
-	ctx2   ossl.EVP_MD_CTX_PTR
-	pinner runtime.Pinner
+	ctx2 ossl.EVP_MD_CTX_PTR
 }
 
 func newEvpHash(ch crypto.Hash) *evpHash {
@@ -318,8 +312,6 @@ func (h *evpHash) Write(p []byte) (int, error) {
 	if len(p) == 0 {
 		return 0, nil
 	}
-	defer h.pinner.Unpin()
-	h.pinner.Pin(&p[0])
 	h.init()
 	if _, err := ossl.EVP_DigestUpdate(h.ctx, pbase(p), len(p)); err != nil {
 		panic(err)

hash_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding"
 	"hash"
 	"io"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -261,6 +262,7 @@ func TestHash_StringWriter(t *testing.T) {
 			}
 			h := cryptoToHash(ch)()
 			initSum := h.Sum(nil)
+			h.(io.StringWriter).WriteString("")
 			h.(io.StringWriter).WriteString(string(msg))
 			h.Reset()
 			sum := h.Sum(nil)
@@ -327,6 +329,7 @@ func TestHash_OneShot(t *testing.T) {
 			if !openssl.SupportsHash(tt.h) {
 				t.Skip("not supported")
 			}
+			_ = tt.oneShot(nil) // test that does not panic
 			got := tt.oneShot(msg)
 			h := cryptoToHash(tt.h)()
 			h.Write(msg)
@@ -382,6 +385,29 @@ func TestHashAllocations(t *testing.T) {
 	}
 }
 
+func verifySHA256(token, salt string) [32]byte {
+	return openssl.SHA256([]byte(token + salt))
+}
+
+func TestIssue71943(t *testing.T) {
+	// https://github.com/golang/go/issues/71943
+	if Asan() {
+		t.Skip("skipping allocations test with sanitizers")
+	}
+	n := int(testing.AllocsPerRun(10, func() {
+		runtime.KeepAlive(verifySHA256("teststring", "test"))
+	}))
+	want := 2
+	if compareCurrentVersion("go1.25") >= 0 {
+		want = 0
+	} else if compareCurrentVersion("go1.24") >= 0 {
+		want = 1
+	}
+	if n > want {
+		t.Errorf("allocs = %d, want %d", n, want)
+	}
+}
+
 func BenchmarkSHA256(b *testing.B) {
 	b.StopTimer()
 	h := openssl.NewSHA256()

internal/mkcgo/mkcgo.go

@@ -34,13 +34,14 @@ type Enum struct {
 
 // FuncAttrs contains attributes of a function.
 type FuncAttrs struct {
-	Tags           []TagAttr
-	VariadicTarget string
-	Optional       bool
-	NoError        bool
-	ErrCond        string
-	NoEscape       bool
-	NoCallback     bool
+	Tags             []TagAttr
+	VariadicTarget   string
+	Optional         bool
+	NoError          bool
+	ErrCond          string
+	NoEscape         bool
+	NoCallback       bool
+	NoCheckPtrParams []string
 }
 
 func (attrs *FuncAttrs) String() string {
@@ -70,6 +71,16 @@ func (attrs *FuncAttrs) String() string {
 	if attrs.NoCallback {
 		bld.WriteString(", nocallback")
 	}
+	if len(attrs.NoCheckPtrParams) != 0 {
+		bld.WriteString(", nocheckptr(")
+		for i, p := range attrs.NoCheckPtrParams {
+			if i > 0 {
+				bld.WriteString(", ")
+			}
+			bld.WriteString(p)
+		}
+		bld.WriteByte(')')
+	}
 	return strings.TrimPrefix(bld.String(), ", ")
 }
 
@@ -116,7 +127,7 @@ func (f *Func) String() string {
 	}
 	bld.WriteString(")")
 	if attrs := f.FuncAttrs.String(); attrs != "" {
-		bld.WriteString(" ")
+		bld.WriteByte(' ')
 		bld.WriteString(attrs)
 	}
 	return bld.String()
@@ -231,4 +242,12 @@ var attributes = [...]attribute{
 			return nil
 		},
 	},
+	{
+		name:        "nocheckptr",
+		description: "The parameter will be hidden from the Go compiler.",
+		handle: func(opts *FuncAttrs, s ...string) error {
+			opts.NoCheckPtrParams = append(opts.NoCheckPtrParams, s[0])
+			return nil
+		},
+	},
 }

internal/mkcgo/mkcgo_test.go

@@ -177,15 +177,16 @@ func TestFuncString(t *testing.T) {
 			name: "Function with attributes",
 			function: &mkcgo.Func{Name: "TestFunc", Ret: "void", Params: []*mkcgo.Param{{Type: "int", Name: "param1"}},
 				FuncAttrs: mkcgo.FuncAttrs{
-					Tags:       []mkcgo.TagAttr{{Tag: "tag1"}, {Tag: "tag2", Name: "name"}},
-					Optional:   true,
-					NoError:    true,
-					ErrCond:    "error_condition",
-					NoEscape:   true,
-					NoCallback: true,
+					Tags:             []mkcgo.TagAttr{{Tag: "tag1"}, {Tag: "tag2", Name: "name"}},
+					Optional:         true,
+					NoError:          true,
+					ErrCond:          "error_condition",
+					NoEscape:         true,
+					NoCallback:       true,
+					NoCheckPtrParams: []string{"param1"},
 				},
 			},
-			want: "void TestFunc(int param1) [{tag1 } {tag2 name}], optional, noerror, errcond(error_condition), noescape, nocallback",
+			want: "void TestFunc(int param1) [{tag1 } {tag2 name}], optional, noerror, errcond(error_condition), noescape, nocallback, nocheckptr(param1)",
 		},
 	}
 
@@ -247,7 +248,7 @@ enum {
 		}, {
 			content: `
 void F0(void) __attribute__((tag("t0"),noerror,tag("t1","tn0")));
-int F1(int p1) __attribute__((errcond("ec0"),  noescape,    nocallback));
+int F1(int p1) __attribute__((errcond("ec0"),  noescape,    nocallback, nocheckptr(p1)));
 int * F2(int **p1, void  * p2);
 int *F3(int p1, void***) __attribute__((optional));
 unsigned   long F4(int func, int type, ...);
@@ -256,7 +257,7 @@ void F6() __attribute__(());`,
 			want: &mkcgo.Source{
 				Funcs: []*mkcgo.Func{
 					{Name: "F0", Ret: "void", Params: []*mkcgo.Param{{"void", ""}}, FuncAttrs: mkcgo.FuncAttrs{Tags: []mkcgo.TagAttr{{Tag: "t0"}, {Tag: "t1", Name: "tn0"}}, NoError: true}},
-					{Name: "F1", Ret: "int", Params: []*mkcgo.Param{{"int", "p1"}}, FuncAttrs: mkcgo.FuncAttrs{ErrCond: "ec0", NoEscape: true, NoCallback: true}},
+					{Name: "F1", Ret: "int", Params: []*mkcgo.Param{{"int", "p1"}}, FuncAttrs: mkcgo.FuncAttrs{ErrCond: "ec0", NoEscape: true, NoCallback: true, NoCheckPtrParams: []string{"p1"}}},
 					{Name: "F2", Ret: "int*", Params: []*mkcgo.Param{{"int**", "p1"}, {"void*", "p2"}}},
 					{Name: "F3", Ret: "int*", Params: []*mkcgo.Param{{"int", "p1"}, {"void***", ""}}, FuncAttrs: mkcgo.FuncAttrs{Optional: true}},
 					{Name: "F4", Ret: "unsigned long", Params: []*mkcgo.Param{{"int", "__func"}, {"int", "__type"}, {"...", ""}}},

internal/ossl/shims.h

@@ -189,10 +189,10 @@ _EVP_MD_CTX_PTR EVP_MD_CTX_new(void);
 void EVP_MD_CTX_free(_EVP_MD_CTX_PTR ctx);
 int EVP_MD_CTX_copy(_EVP_MD_CTX_PTR out, const _EVP_MD_CTX_PTR in);
 int EVP_MD_CTX_copy_ex(_EVP_MD_CTX_PTR out, const _EVP_MD_CTX_PTR in);
-int EVP_Digest(const void *data, size_t count, unsigned char *md, unsigned int *size, const _EVP_MD_PTR type, _ENGINE_PTR impl) __attribute__((noescape,nocallback));
+int EVP_Digest(const void *data, size_t count, unsigned char *md, unsigned int *size, const _EVP_MD_PTR type, _ENGINE_PTR impl) __attribute__((noescape,nocallback,nocheckptr("data")));
 int EVP_DigestInit_ex(_EVP_MD_CTX_PTR ctx, const _EVP_MD_PTR type, _ENGINE_PTR impl);
 int EVP_DigestInit(_EVP_MD_CTX_PTR ctx, const _EVP_MD_PTR type);
-int EVP_DigestUpdate(_EVP_MD_CTX_PTR ctx, const void *d, size_t cnt) __attribute__((noescape,nocallback));
+int EVP_DigestUpdate(_EVP_MD_CTX_PTR ctx, const void *d, size_t cnt) __attribute__((noescape,nocallback,nocheckptr("d")));
 int EVP_DigestFinal_ex(_EVP_MD_CTX_PTR ctx, unsigned char *md, unsigned int *s);
 int EVP_DigestSign(_EVP_MD_CTX_PTR ctx, unsigned char *sigret, size_t *siglen, const unsigned char *tbs, size_t tbslen) __attribute__((tag("111"),noescape,nocallback));
 int EVP_DigestSignInit(_EVP_MD_CTX_PTR ctx, _EVP_PKEY_CTX_PTR *pctx, const _EVP_MD_PTR type, _ENGINE_PTR e, _EVP_PKEY_PTR pkey);

internal/ossl/zossl.go

@@ -252,6 +252,12 @@ func baseNeverEmpty(b []byte) *byte {
 	return unsafe.SliceData(b)
 }
 
+// pbaseNeverEmpty returns the address of the underlying array in b.
+// If b has zero length, it returns a pointer to a zero byte.
+func pbaseNeverEmpty(b []byte) unsafe.Pointer {
+	return unsafe.Pointer(baseNeverEmpty(b))
+}
+
 // pbase returns the address of the underlying array in b,
 // being careful not to panic when b has zero length.
 func pbase(b []byte) unsafe.Pointer {