From: qmuntal <quimmuntal@gmail.com>
Date: Mon, 18 Nov 2024 08:31:59 +0000 (+0100)
Subject: os/user: fix race detector failure
X-Git-Tag: go1.24rc1~270
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=405a0c4ae86fe2761118ee6d1d59e59daf9b50cd;p=gostls13.git

os/user: fix race detector failure

The race detector doesn't like that windows.GetSid* functions return
pointers to the SID structure. This change makes these functions return
values instead and mark them with nocheckptr.

Fixes #70378

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-race
Change-Id: Iba39d75bb31679d25a5ee43b51e4abb0c435dbac
Reviewed-on: https://go-review.googlesource.com/c/go/+/628995
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---

diff --git a/src/internal/syscall/windows/security_windows.go b/src/internal/syscall/windows/security_windows.go
index 547c30031a..017e25aaef 100644
--- a/src/internal/syscall/windows/security_windows.go
+++ b/src/internal/syscall/windows/security_windows.go
@@ -5,6 +5,7 @@
 package windows
 
 import (
+	"runtime"
 	"syscall"
 	"unsafe"
 )
@@ -231,6 +232,33 @@ var SECURITY_NT_AUTHORITY = SID_IDENTIFIER_AUTHORITY{
 }
 
 //sys	IsValidSid(sid *syscall.SID) (valid bool) = advapi32.IsValidSid
-//sys	GetSidIdentifierAuthority(sid *syscall.SID) (idauth *SID_IDENTIFIER_AUTHORITY) = advapi32.GetSidIdentifierAuthority
-//sys	GetSidSubAuthority(sid *syscall.SID, subAuthorityIdx uint32) (subAuth *uint32) = advapi32.GetSidSubAuthority
-//sys	GetSidSubAuthorityCount(sid *syscall.SID) (count *uint8) = advapi32.GetSidSubAuthorityCount
+//sys	getSidIdentifierAuthority(sid *syscall.SID) (idauth uintptr) = advapi32.GetSidIdentifierAuthority
+//sys	getSidSubAuthority(sid *syscall.SID, subAuthorityIdx uint32) (subAuth uintptr) = advapi32.GetSidSubAuthority
+//sys	getSidSubAuthorityCount(sid *syscall.SID) (count uintptr) = advapi32.GetSidSubAuthorityCount
+
+// The following GetSid* functions are marked as //go:nocheckptr because checkptr
+// instrumentation can't see that the pointer returned by the syscall is pointing
+// into the sid's memory, which is normally allocated on the Go heap. Therefore,
+// the checkptr instrumentation would incorrectly flag the pointer dereference
+// as pointing to an invalid allocation.
+// Also, use runtime.KeepAlive to ensure that the sid is not garbage collected
+// before the GetSid* functions return, as the Go GC is not aware that the
+// pointers returned by the syscall are pointing into the sid's memory.
+
+//go:nocheckptr
+func GetSidIdentifierAuthority(sid *syscall.SID) SID_IDENTIFIER_AUTHORITY {
+	defer runtime.KeepAlive(sid)
+	return *(*SID_IDENTIFIER_AUTHORITY)(unsafe.Pointer(getSidIdentifierAuthority(sid)))
+}
+
+//go:nocheckptr
+func GetSidSubAuthority(sid *syscall.SID, subAuthorityIdx uint32) uint32 {
+	defer runtime.KeepAlive(sid)
+	return *(*uint32)(unsafe.Pointer(getSidSubAuthority(sid, subAuthorityIdx)))
+}
+
+//go:nocheckptr
+func GetSidSubAuthorityCount(sid *syscall.SID) uint8 {
+	defer runtime.KeepAlive(sid)
+	return *(*uint8)(unsafe.Pointer(getSidSubAuthorityCount(sid)))
+}
diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go
index f4048c440e..c81bc399ff 100644
--- a/src/internal/syscall/windows/zsyscall_windows.go
+++ b/src/internal/syscall/windows/zsyscall_windows.go
@@ -124,21 +124,21 @@ func DuplicateTokenEx(hExistingToken syscall.Token, dwDesiredAccess uint32, lpTo
 	return
 }
 
-func GetSidIdentifierAuthority(sid *syscall.SID) (idauth *SID_IDENTIFIER_AUTHORITY) {
+func getSidIdentifierAuthority(sid *syscall.SID) (idauth uintptr) {
 	r0, _, _ := syscall.Syscall(procGetSidIdentifierAuthority.Addr(), 1, uintptr(unsafe.Pointer(sid)), 0, 0)
-	idauth = (*SID_IDENTIFIER_AUTHORITY)(unsafe.Pointer(r0))
+	idauth = uintptr(r0)
 	return
 }
 
-func GetSidSubAuthority(sid *syscall.SID, subAuthorityIdx uint32) (subAuth *uint32) {
+func getSidSubAuthority(sid *syscall.SID, subAuthorityIdx uint32) (subAuth uintptr) {
 	r0, _, _ := syscall.Syscall(procGetSidSubAuthority.Addr(), 2, uintptr(unsafe.Pointer(sid)), uintptr(subAuthorityIdx), 0)
-	subAuth = (*uint32)(unsafe.Pointer(r0))
+	subAuth = uintptr(r0)
 	return
 }
 
-func GetSidSubAuthorityCount(sid *syscall.SID) (count *uint8) {
+func getSidSubAuthorityCount(sid *syscall.SID) (count uintptr) {
 	r0, _, _ := syscall.Syscall(procGetSidSubAuthorityCount.Addr(), 1, uintptr(unsafe.Pointer(sid)), 0, 0)
-	count = (*uint8)(unsafe.Pointer(r0))
+	count = uintptr(r0)
 	return
 }
 
diff --git a/src/os/user/lookup_windows.go b/src/os/user/lookup_windows.go
index 11bb58e87b..e0e77f3ea7 100644
--- a/src/os/user/lookup_windows.go
+++ b/src/os/user/lookup_windows.go
@@ -98,11 +98,11 @@ func isServiceAccount(sid *syscall.SID) bool {
 	// - "S-1-5-18": LocalSystem
 	// - "S-1-5-19": LocalService
 	// - "S-1-5-20": NetworkService
-	if *windows.GetSidSubAuthorityCount(sid) != windows.SID_REVISION ||
-		*windows.GetSidIdentifierAuthority(sid) != windows.SECURITY_NT_AUTHORITY {
+	if windows.GetSidSubAuthorityCount(sid) != windows.SID_REVISION ||
+		windows.GetSidIdentifierAuthority(sid) != windows.SECURITY_NT_AUTHORITY {
 		return false
 	}
-	switch *windows.GetSidSubAuthority(sid, 0) {
+	switch windows.GetSidSubAuthority(sid, 0) {
 	case windows.SECURITY_LOCAL_SYSTEM_RID,
 		windows.SECURITY_LOCAL_SERVICE_RID,
 		windows.SECURITY_NETWORK_SERVICE_RID: