internal/syscall/windows: fix the signature of SetFileInformationByHandle

Bryan C. Mills · gopherbot · commit a7097243e462 · 2023-12-18T19:20:32.000Z
Also fix its call site in internal/poll to pass the length of the actual buffer instead of an unrelated variable, and update the definition of FILE_BASIC_INFO to match the documented field types and add padding that is empirically needed on the 386 architecture. Passing a pointer to a Go-allocated buffer as type uintptr violates the unsafe.Pointer conversion rules, which allow such a conversion only in the call expression itself for a call to syscall.Syscall or equivalent. That can allow the buffer to be corrupted arbitrarily if the Go runtime happens to garbage-collect it while the call to SetFileInformationByHandle is in progress. The Microsoft documentation for SetFileInformationByHandle specifies its third argument type as LPVOID, which corresponds to Go's unsafe.Pointer, not uintptr. Fixes #58933 (maybe). Change-Id: If577b57adea9922f5fcca55e46030c703d8f035c Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/549256 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Than McIntosh <thanm@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go
@@ -1037,8 +1037,7 @@ func (fd *FD) Fchmod(mode uint32) error {
 
 	var du windows.FILE_BASIC_INFO
 	du.FileAttributes = attrs
-	l := uint32(unsafe.Sizeof(d))
-	return windows.SetFileInformationByHandle(fd.Sysfd, windows.FileBasicInfo, uintptr(unsafe.Pointer(&du)), l)
+	return windows.SetFileInformationByHandle(fd.Sysfd, windows.FileBasicInfo, unsafe.Pointer(&du), uint32(unsafe.Sizeof(du)))
 }
 
 // Fchdir wraps syscall.Fchdir.
diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go
@@ -129,11 +129,22 @@ type SecurityAttributes struct {
 }
 
 type FILE_BASIC_INFO struct {
-	CreationTime   syscall.Filetime
-	LastAccessTime syscall.Filetime
-	LastWriteTime  syscall.Filetime
-	ChangedTime    syscall.Filetime
+	CreationTime   int64
+	LastAccessTime int64
+	LastWriteTime  int64
+	ChangedTime    int64
 	FileAttributes uint32
+
+	// Pad out to 8-byte alignment.
+	//
+	// Without this padding, TestChmod fails due to an argument validation error
+	// in SetFileInformationByHandle on windows/386.
+	//
+	// https://learn.microsoft.com/en-us/cpp/build/reference/zp-struct-member-alignment?view=msvc-170
+	// says that “The C/C++ headers in the Windows SDK assume the platform's
+	// default alignment is used.” What we see here is padding rather than
+	// alignment, but maybe it is related.
+	_ uint32
 }
 
 const (
@@ -150,7 +161,7 @@ const (
 //sys	GetComputerNameEx(nameformat uint32, buf *uint16, n *uint32) (err error) = GetComputerNameExW
 //sys	MoveFileEx(from *uint16, to *uint16, flags uint32) (err error) = MoveFileExW
 //sys	GetModuleFileName(module syscall.Handle, fn *uint16, len uint32) (n uint32, err error) = kernel32.GetModuleFileNameW
-//sys	SetFileInformationByHandle(handle syscall.Handle, fileInformationClass uint32, buf uintptr, bufsize uint32) (err error) = kernel32.SetFileInformationByHandle
+//sys	SetFileInformationByHandle(handle syscall.Handle, fileInformationClass uint32, buf unsafe.Pointer, bufsize uint32) (err error) = kernel32.SetFileInformationByHandle
 //sys	VirtualQuery(address uintptr, buffer *MemoryBasicInformation, length uintptr) (err error) = kernel32.VirtualQuery
 //sys	GetTempPath2(buflen uint32, buf *uint16) (n uint32, err error) = GetTempPath2W
 
diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go

Original file line number	Diff line number	Diff line change
`@@ -1037,8 +1037,7 @@ func (fd *FD) Fchmod(mode uint32) error {`
`1037`	`1037`
`1038`	`1038`	`var du windows.FILE_BASIC_INFO`
`1039`	`1039`	`du.FileAttributes = attrs`
`1040`		`- l := uint32(unsafe.Sizeof(d))`
`1041`		`- return windows.SetFileInformationByHandle(fd.Sysfd, windows.FileBasicInfo, uintptr(unsafe.Pointer(&du)), l)`
	`1040`	`+ return windows.SetFileInformationByHandle(fd.Sysfd, windows.FileBasicInfo, unsafe.Pointer(&du), uint32(unsafe.Sizeof(du)))`
`1042`	`1041`	`}`
`1043`	`1042`
`1044`	`1043`	`// Fchdir wraps syscall.Fchdir.`