From fa603aaa4e7ac460aee2e2de509842b89152a418 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 5 May 2022 10:16:30 -0400 Subject: [PATCH] crypto/internal/boring: avoid false positive in cgo pointer check in SHA calls Discovered running recent changes against Google internal tests. Change-Id: Ief51eae82c9f27d2a2a70c4fb2b1086fa8b3f9d8 Reviewed-on: https://go-review.googlesource.com/c/go/+/404295 Auto-Submit: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Run-TryBot: Russ Cox --- src/crypto/internal/boring/boring.go | 14 +++++++++++ src/crypto/internal/boring/sha.go | 35 +++++++++++++++------------- src/crypto/sha256/sha256_test.go | 15 ++++++++++++ 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/crypto/internal/boring/boring.go b/src/crypto/internal/boring/boring.go index d46166e4e1..c560679192 100644 --- a/src/crypto/internal/boring/boring.go +++ b/src/crypto/internal/boring/boring.go @@ -107,3 +107,17 @@ func noescape(p unsafe.Pointer) unsafe.Pointer { x := uintptr(p) return unsafe.Pointer(x ^ 0) } + +var zero byte + +// addr converts p to its base addr, including a noescape along the way. +// If p is nil, addr returns a non-nil pointer, so that the result can always +// be dereferenced. +// +//go:nosplit +func addr(p []byte) *byte { + if len(p) == 0 { + return &zero + } + return (*byte)(noescape(unsafe.Pointer(&p[0]))) +} diff --git a/src/crypto/internal/boring/sha.go b/src/crypto/internal/boring/sha.go index 5fa3db57f8..15b50c90d3 100644 --- a/src/crypto/internal/boring/sha.go +++ b/src/crypto/internal/boring/sha.go @@ -63,43 +63,46 @@ import ( "unsafe" ) -func addr(p []byte) unsafe.Pointer { - if len(p) == 0 { - return nil - } - return unsafe.Pointer(&p[0]) -} +// NOTE: The cgo calls in this file are arranged to avoid marking the parameters as escaping. +// To do that, we call noescape (including via addr). +// We must also make sure that the data pointer arguments have the form unsafe.Pointer(&...) +// so that cgo does not annotate them with cgoCheckPointer calls. If it did that, it might look +// beyond the byte slice and find Go pointers in unprocessed parts of a larger allocation. +// To do both of these simultaneously, the idiom is unsafe.Pointer(&*addr(p)), +// where addr returns the base pointer of p, substituting a non-nil pointer for nil, +// and applying a noescape along the way. +// This is all to preserve compatibility with the allocation behavior of the non-boring implementations. func SHA1(p []byte) (sum [20]byte) { - if C._goboringcrypto_gosha1(noescape(addr(p)), C.size_t(len(p)), noescape(unsafe.Pointer(&sum[0]))) == 0 { + if C._goboringcrypto_gosha1(unsafe.Pointer(&*addr(p)), C.size_t(len(p)), unsafe.Pointer(&*addr(sum[:]))) == 0 { panic("boringcrypto: SHA1 failed") } return } func SHA224(p []byte) (sum [28]byte) { - if C._goboringcrypto_gosha224(noescape(addr(p)), C.size_t(len(p)), noescape(unsafe.Pointer(&sum[0]))) == 0 { + if C._goboringcrypto_gosha224(unsafe.Pointer(&*addr(p)), C.size_t(len(p)), unsafe.Pointer(&*addr(sum[:]))) == 0 { panic("boringcrypto: SHA224 failed") } return } func SHA256(p []byte) (sum [32]byte) { - if C._goboringcrypto_gosha256(noescape(addr(p)), C.size_t(len(p)), noescape(unsafe.Pointer(&sum[0]))) == 0 { + if C._goboringcrypto_gosha256(unsafe.Pointer(&*addr(p)), C.size_t(len(p)), unsafe.Pointer(&*addr(sum[:]))) == 0 { panic("boringcrypto: SHA256 failed") } return } func SHA384(p []byte) (sum [48]byte) { - if C._goboringcrypto_gosha384(noescape(addr(p)), C.size_t(len(p)), noescape(unsafe.Pointer(&sum[0]))) == 0 { + if C._goboringcrypto_gosha384(unsafe.Pointer(&*addr(p)), C.size_t(len(p)), unsafe.Pointer(&*addr(sum[:]))) == 0 { panic("boringcrypto: SHA384 failed") } return } func SHA512(p []byte) (sum [64]byte) { - if C._goboringcrypto_gosha512(noescape(addr(p)), C.size_t(len(p)), noescape(unsafe.Pointer(&sum[0]))) == 0 { + if C._goboringcrypto_gosha512(unsafe.Pointer(&*addr(p)), C.size_t(len(p)), unsafe.Pointer(&*addr(sum[:]))) == 0 { panic("boringcrypto: SHA512 failed") } return @@ -137,7 +140,7 @@ func (h *sha1Hash) BlockSize() int { return 64 } func (h *sha1Hash) Sum(dst []byte) []byte { return h.sum(dst) } func (h *sha1Hash) Write(p []byte) (int, error) { - if len(p) > 0 && C._goboringcrypto_SHA1_Update(h.noescapeCtx(), noescape(addr(p)), C.size_t(len(p))) == 0 { + if len(p) > 0 && C._goboringcrypto_SHA1_Update(h.noescapeCtx(), unsafe.Pointer(&*addr(p)), C.size_t(len(p))) == 0 { panic("boringcrypto: SHA1_Update failed") } return len(p), nil @@ -217,7 +220,7 @@ func (h *sha224Hash) BlockSize() int { return 64 } func (h *sha224Hash) Sum(dst []byte) []byte { return h.sum(dst) } func (h *sha224Hash) Write(p []byte) (int, error) { - if len(p) > 0 && C._goboringcrypto_SHA224_Update(h.noescapeCtx(), noescape(addr(p)), C.size_t(len(p))) == 0 { + if len(p) > 0 && C._goboringcrypto_SHA224_Update(h.noescapeCtx(), unsafe.Pointer(&*addr(p)), C.size_t(len(p))) == 0 { panic("boringcrypto: SHA224_Update failed") } return len(p), nil @@ -255,7 +258,7 @@ func (h *sha256Hash) BlockSize() int { return 64 } func (h *sha256Hash) Sum(dst []byte) []byte { return h.sum(dst) } func (h *sha256Hash) Write(p []byte) (int, error) { - if len(p) > 0 && C._goboringcrypto_SHA256_Update(h.noescapeCtx(), noescape(addr(p)), C.size_t(len(p))) == 0 { + if len(p) > 0 && C._goboringcrypto_SHA256_Update(h.noescapeCtx(), unsafe.Pointer(&*addr(p)), C.size_t(len(p))) == 0 { panic("boringcrypto: SHA256_Update failed") } return len(p), nil @@ -392,7 +395,7 @@ func (h *sha384Hash) BlockSize() int { return 128 } func (h *sha384Hash) Sum(dst []byte) []byte { return h.sum(dst) } func (h *sha384Hash) Write(p []byte) (int, error) { - if len(p) > 0 && C._goboringcrypto_SHA384_Update(h.noescapeCtx(), noescape(addr(p)), C.size_t(len(p))) == 0 { + if len(p) > 0 && C._goboringcrypto_SHA384_Update(h.noescapeCtx(), unsafe.Pointer(&*addr(p)), C.size_t(len(p))) == 0 { panic("boringcrypto: SHA384_Update failed") } return len(p), nil @@ -430,7 +433,7 @@ func (h *sha512Hash) BlockSize() int { return 128 } func (h *sha512Hash) Sum(dst []byte) []byte { return h.sum(dst) } func (h *sha512Hash) Write(p []byte) (int, error) { - if len(p) > 0 && C._goboringcrypto_SHA512_Update(h.noescapeCtx(), noescape(addr(p)), C.size_t(len(p))) == 0 { + if len(p) > 0 && C._goboringcrypto_SHA512_Update(h.noescapeCtx(), unsafe.Pointer(&*addr(p)), C.size_t(len(p))) == 0 { panic("boringcrypto: SHA512_Update failed") } return len(p), nil diff --git a/src/crypto/sha256/sha256_test.go b/src/crypto/sha256/sha256_test.go index de807c9b07..7304678346 100644 --- a/src/crypto/sha256/sha256_test.go +++ b/src/crypto/sha256/sha256_test.go @@ -310,6 +310,21 @@ func TestAllocations(t *testing.T) { } } +type cgoData struct { + Data [16]byte + Ptr *cgoData +} + +func TestCgo(t *testing.T) { + // Test that Write does not cause cgo to scan the entire cgoData struct for pointers. + // The scan (if any) should be limited to the [16]byte. + d := new(cgoData) + d.Ptr = d + h := New() + h.Write(d.Data[:]) + h.Sum(nil) +} + var bench = New() var buf = make([]byte, 8192) -- 2.48.1