Self-relative security descriptors can sometimes be smaller than the
header size of the SECURITY_DESCRIPTOR struct, which is fine in C, but
checkptr complains about it, so instead, ensure that the allocation is
at least the size of the SECURITY_DESCRIPTOR struct.
This fixes this splat:
fatal error: checkptr: converted pointer straddles multiple allocations
goroutine 36 [running]:
runtime.throw(0x761c5f, 0x3a)
/usr/lib/go/src/runtime/panic.go:1117 +0x79 fp=0xc00005dd90 sp=0xc00005dd60 pc=0x5fb8d9
runtime.checkptrAlignment(0xc0001a4020, 0x741860, 0x1)
/usr/lib/go/src/runtime/checkptr.go:20 +0xc9 fp=0xc00005ddc0 sp=0xc00005dd90 pc=0x5c7729
golang.org/x/sys/windows.(*SECURITY_DESCRIPTOR).copySelfRelativeSecurityDescriptor(0x1b9ad638190, 0x2)
/home/zx2c4/Projects/golang-dev/sys/windows/security_windows.go:1359 +0x1b7 fp=0xc00005de50 sp=0xc00005ddc0 pc=0x6f0077
golang.org/x/sys/windows.SecurityDescriptorFromString(0x754f2b, 0x2, 0x0, 0x0, 0x0)
/home/zx2c4/Projects/golang-dev/sys/windows/security_windows.go:1371 +0xde fp=0xc00005deb8 sp=0xc00005de50 pc=0x6f019e
Change-Id: I552017a93d4ca6f6debc6f8f445dac5c6717fed1
Reviewed-on: https://go-review.googlesource.com/c/sys/+/307129
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This struct has pointers in it, which means checkptr expects for it to
be aligned properly. When we're copying a Windows-allocated struct to a
Go-allocated one, make sure that the Go allocation is aligned to the
pointer size.
This fixes the following checkptr splat:
goroutine 29 [running]:
runtime.throw(0x4f8dd9, 0x3a)
C:/hostedtoolcache/windows/go/1.16.2/x64/src/runtime/panic.go:1117 +0x79 fp=0xc000041c50 sp=0xc000041c20 pc=0x2f9879
runtime.checkptrAlignment(0xc00009c180, 0x4d8a00, 0x1)
C:/hostedtoolcache/windows/go/1.16.2/x64/src/runtime/checkptr.go:20 +0xc9 fp=0xc000041c80 sp=0xc000041c50 pc=0x2c4d09
golang.org/x/sys/windows.(*SECURITY_DESCRIPTOR).copySelfRelativeSecurityDescriptor(0x227284caa00, 0x2)
C:/Users/runneradmin/go/pkg/mod/golang.org/x/sys@v0.0.0-20210309040221-94ec62e08169/windows/security_windows.go:1347 +0x11f fp=0xc000041ce8 sp=0xc000041c80 pc=0x467f9f
golang.org/x/sys/windows.SecurityDescriptorFromString(0x4ebfb0, 0x2, 0x0, 0x0, 0x0)
Change-Id: I7eb9c07e7afb7f139473b660f82a23541663ec50
Reviewed-on: https://go-review.googlesource.com/c/sys/+/306889
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Alex copied this from a temporary thing I had written for the runtime
package. In runtime, you can't really access other packages like
syscall, so everything has to be very manual. But in x/sys, we can do
things properly. So this reimplements the function in a more straight
forward way.
Change-Id: I1634904bb1e10f33252954ce02d4b17ae56592e5
Reviewed-on: https://go-review.googlesource.com/c/sys/+/298830
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In anticipation of the next commit which adds win32 pipe APIs, add some
of the foundational NT APIs for that, which will be required for making
a robust Go pipe library. Also add a simple test case.
Change-Id: I898bd6c5265a8939a7f05a24c4d9b22941dc56b7
Reviewed-on: https://go-review.googlesource.com/c/sys/+/298171
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It turns out that the proc thread update function doesn't actually
allocate new memory for its arguments and instead just copies the
pointer values into the preallocated memory. Since we were allocating
that memory as []byte, the garbage collector didn't scan it for pointers
to Go allocations and freed them. We _could_ fix this by requiring that
all users of this use runtime.KeepAlive for everything they pass to the
update function, but that seems harder than necessary. Instead, we can
just do the allocation as []unsafe.Pointer, which means the GC can
operate as intended and not free these from beneath our feet. In order
to ensure this remains true, we also add a test for this.
Updates golang/go#44662.
Change-Id: Iaa8b694a6682cc1876879632c7ba068e47b8666d
Reviewed-on: https://go-review.googlesource.com/c/sys/+/297331
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The prior function updated the TEB's LastStatus member, which is not
what we want to be doing here. It's also not consistent with Microsoft's
own Go code for their pipe library, which properly uses the "NoTeb"
variant like this commit.
For good measure, we add a simple test case to make sure these paths are
being exercised.
Change-Id: I4080898f704bdc93a6048001b06ffce516fb412d
Reviewed-on: https://go-review.googlesource.com/c/sys/+/298169
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
All references to "Errno" go through the syscalldot function, which
prepends "syscall." if the generation target is not in the syscall
package. Similarly so, we need a windowsdot function to prepend
"windows." to NTStatus.
Change-Id: I86affcf0155632ed5848ebabca97d175a041c6c3
Reviewed-on: https://go-review.googlesource.com/c/sys/+/298170
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The same symbol name might be defined multiple times with different
function names or signatures. In that case, it's an error to redefine
the proc variable. So deduplicate these before printing.
Change-Id: I7a636cd44fb1ea6f51840ef3924d6316e266faa0
Reviewed-on: https://go-review.googlesource.com/c/sys/+/288332
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The native NT API returns error values from a different namespace as the
usual Win32 one. This means it needs to be typed differently. This
commit adds broad support for using NTSTATUS values in a new type called
NTStatus.
First we add the type as a basic uint32. Then we add all of the
predefined constants from ntstatus.h, by augmenting mkerrors.bash to do
automatic extraction. There's a convenece way to convert an NT error to
a Win32 error, so we add the NTStatus.Errno() function. Since NTStatus
is an error type, we define an Error() function that returns a string by
asking ntdll.dll for its contents, in the exact same way that
syscall.Errno.Error() does, by calling FormatMessage. Since functions
need to actually use this, we add the rule that if a `//sys` declaration
returns an error value called "ntstatus", then the type underlying the
error interface is an NTStatus instead of an Errno. Finally we fix one
function that was returning an error interface of an Errno rather than
an NTStatus.
Change-Id: I06296b9563bbec526759d12a19f13ac6ad46dcc3
Reviewed-on: https://go-review.googlesource.com/c/sys/+/297330
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The third argument to GetQueuedCompletionStatus is a pointer to a
uintptr, not a uint32. Users of this functions have therefore been
corrupting their memory every time they used it. Either that memory
corruption was silent (dangerous), or their programs didn't work so they
chose a different API to use.
Updates golang/go#44538.
Change-Id: Ie1f66de11001cf9c8195afaa61f003a86f821a95
Reviewed-on: https://go-review.googlesource.com/c/sys/+/295174
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
This fixes the name of a struct to match Microsoft's documentation.
It also reverts a change I made in the last patchset of CL 285714. The
idea there was that a Go bool would suffice for a win32 BOOL, because of
little endian. But in fact, a value of 0xff000000 would be treated as
false by Go but true by C, which is a problem. So this changes to using
a vanilla int32 type, which matches the C "int" used in the Microsoft
headers.
Change-Id: Id7cd306e916b3754e8dfe32bf11ec30cad3a13b4
Reviewed-on: https://go-review.googlesource.com/c/sys/+/285717
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This function returns 0 and sets last error on failure. While this isn't
said explicitly on MSDN, there's no PID 0, and trivial reverse
engineering shows that this is the case. For example:
.text:00000001800035ED loc_1800035ED: ; CODE XREF: GetWindowThreadProcessId+23↑j
.text:00000001800035ED ; GetWindowThreadProcessId+3D↑j ...
.text:00000001800035ED mov ecx, 578h ; LastError
.text:00000001800035F2 call cs:__imp_RtlSetLastWin32Error
.text:00000001800035F9 nop dword ptr [rax+rax+00h]
.text:00000001800035FE
.text:00000001800035FE loc_1800035FE: ; CODE XREF: GetWindowThreadProcessId+65↑j
.text:00000001800035FE xor eax, eax
.text:0000000180003600 jmp short loc_1800035AA
This function was also just added by a user who is likely its only
consumer, so this error is not too late to fix.
Change-Id: I5dd24e78c006686bb8f1288ad0fe63cd67df56a6
Reviewed-on: https://go-review.googlesource.com/c/sys/+/285272
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This matches usual win32api conventions. While we're at it, we
group together user32.dll functions together.
This CL was based on CL 282634 with all but MessageBox, GetShellWindow
and GetWindowThreadProcessId changes removed to prevent compatibility
break.
Change-Id: I7e17c581723c41580a49c5612cabc7a5c13c0f15
Reviewed-on: https://go-review.googlesource.com/c/sys/+/282972
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Alex Brainman <alex.brainman@gmail.com>
One thing the newer notification API does not do is automatically
provide the service exit code in its notifier response. It requires
querying manually. Unfortunately, we weren't propagating this
information up from the lower level struct, so this commit copys that
information over.
Change-Id: I70c683007ce34ffab6196329acefc8443f921ebe
Reviewed-on: https://go-review.googlesource.com/c/sys/+/274577
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
The "?" code assumed that the error value was always called "err", when
in reality it might be called something different (like "ret") or even
entirely absent. This commit makes the templating robust to that. At the
same time, we move a lot of the complexity out of the actual templates
and into helper functions, so that this remains easy to read.
Change-Id: I939d56413a24f0e3e1bbf13da5adf13e9401747a
Reviewed-on: https://go-review.googlesource.com/c/sys/+/275472
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Currently, doing s.UpdateConfig(s.Config()) will destroy the service,
because s.Config() fails to populate the SidType member, but
UpdateConfig will set the SidType, so that expression effectively zeros
out the SidType. Fix this by having Config() fetch the SidType in the
same way that it fetches the other additional fields there, such as
DelayedStartUp.
Change-Id: Idb917ef1e942020499b411b7777b995c29f0e7d2
Reviewed-on: https://go-review.googlesource.com/c/sys/+/270897
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
