syscall: memory corruption in *bool types generated by mksyscall_windows.go #34364
Comments
|
Fix history as of writing: For x/sys/windows, my first attempt was to just get rid of *bool and pass an explicit *uint32, breaking the API: https://go-review.googlesource.com/c/sys/+/195839 . The first attempt was rejected. Reviewers suggested instead I fix the source of the problem directly, mksyscall_windows.go, which I did: https://go-review.googlesource.com/c/go/+/196122 . Then, I was able to regenerate x/sys/windows using it: https://go-review.googlesource.com/c/sys/+/196123 . |
Windows type PBOOL is a pointer to a 4 byte value, where 0 means false and not-0 means true. That means we should use uint32 here, not bool, since Go bools can be 1 byte. This commit was re-generated using mksyscall_windows.go from CL 196122. Updates: golang/go#34364 Change-Id: I8e83b9a09c0b58d14ac9a7dee316553940ac6ee3
|
Change https://golang.org/cl/196123 mentions this issue: |
|
Change https://golang.org/cl/196122 mentions this issue: |
FiloSottile
changed the title
FiloSottile
added
NeedsFix
Windows type PBOOL is a pointer to a 4 byte value, where 0 means false and not-0 means true. That means we should use uint32 here, not bool, since Go bools can be 1 byte. This commit was re-generated using mksyscall_windows.go from CL 196122. Updates: golang/go#34364 Change-Id: I8e83b9a09c0b58d14ac9a7dee316553940ac6ee3
…arameters Windows type PBOOL is a pointer to a 4 byte value, where 0 means false and not-0 means true. That means we should use uint32 here, not bool, since Go bools can be 1 byte. Since a *bool is never a "real" valid Windows type, converting on both in and out is probably sufficient, since *bool shouldn't ever be used as something with significance for its particular address. Updates: #34364 Change-Id: I4c1b91cd9a39d91e23dae6f894b9a49f7fba2c0a Reviewed-on: https://go-review.googlesource.com/c/go/+/196122 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alex Brainman <[email protected]>
Windows type PBOOL is a pointer to a 4 byte value, where 0 means false and not-0 means true. That means we should use uint32 here, not bool, since Go bools can be 1 byte. This commit was re-generated using mksyscall_windows.go from CL 196122. Updates: golang/go#34364 Change-Id: I8e83b9a09c0b58d14ac9a7dee316553940ac6ee3 Reviewed-on: https://go-review.googlesource.com/c/sys/+/196123 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]>
|
@gopherbot please backport this to 1.13 |
|
Backport issue(s) opened: #34388 (for 1.13). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
|
Change https://golang.org/cl/196417 mentions this issue: |
|
Is 1.12 also affected? |
|
Yes. |
|
@gopherbot please backport this to 1.12 EDIT: not sure why this didn't work... |
@networkimprov You can only make one backport request per issue at this time, see issue #25574. |
This is an issue to track a memory corruption bug in mksyscall_windows.go and its effects various places.
Windows type PBOOL is a pointer to a 4 byte value, where 0 means false and not-0 means true. That means we should use uint32 when passing pointers, not bool, since Go bools are 1 byte, not 4. Previously, we were passing the 1 byte bool to the 4 byte argument, resulting in 3 null bytes being written somewhere bad. I observed this memory corruption in the wild, and it's not pretty.
x/sys/windows currently has one buggy function. However, lots and lots of projects use Go's mksyscall_windows.go -- it was meant for external consumption -- and it's possible that external projects are affected by this. This issue can be used to indicate the problem to these places.
CC @alexbrainman @ianlancetaylor @bradfitz
The text was updated successfully, but these errors were encountered: