Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/poll: invalid uintptr conversion in call to windows.SetFileInformationByHandle [1.21 backport] #65882

Closed
gopherbot opened this issue Feb 22, 2024 · 2 comments
Labels
Builders x/build issues (builders, bots, dashboards) CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. OS-Windows
Milestone

Comments

@gopherbot
Copy link

@bcmills requested issue #58933 to be considered for backport to the next 1.21 minor release.

@gopherbot, please backport to Go 1.21. This was a use-after-free bug and could cause arbitrary heap corruption in programs that call SetFileInformationByHandle, and it also produces nondeterministic test failures on the release-branch.go1.21 builders.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 22, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 22, 2024
@gopherbot gopherbot added this to the Go1.21.8 milestone Feb 22, 2024
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Feb 22, 2024
@gopherbot
Copy link
Author

Change https://go.dev/cl/566155 mentions this issue: [release-branch.go1.21] internal/syscall/windows: fix the signature of SetFileInformationByHandle

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Feb 28, 2024
@gopherbot
Copy link
Author

Closed by merging 3a58877 to release-branch.go1.21.

gopherbot pushed a commit that referenced this issue Feb 28, 2024
…f SetFileInformationByHandle

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 #65882.
Updates #58933.

Change-Id: If577b57adea9922f5fcca55e46030c703d8f035c
Cq-Include-Trybots: luci.golang.try:go1.21-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>
(cherry picked from commit a709724)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566155
Reviewed-by: Bryan Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. OS-Windows
Projects
None yet
Development

No branches or pull requests

3 participants