Commit Graph
20 Commits
Author SHA1 Message Date
Bryan C. Mills 1911637744 windows/svc: use separate (and more descriptive) service names in tests
Notably, the DisplayName field was set to the same thing in both
sys.TestExample and mrg.TestMyService, which may explain the collision
reported in golang/go#59298.

Moreover, the adjective ”my” conveys no information whatsoever — we
shouldn't use it in tests or examples.

Also skip the tests that install services if GO_BUILDER_NAME is not
set, to reduce the likelihood of 'go test all' in a user's working
directory being mistaken for a malicious or compromised program.

Fixes golang/go#59298.

Change-Id: Ib00bf7400bfaa34e1a1d49125c43b97019b53c82
Reviewed-on: https://go-review.googlesource.com/c/sys/+/481015
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
2023-05-04 17:52:42 +00:00
Bryan C. Mills ca59edaa5a windows: use unsafe.Add instead of pointer arithmetic on a uintptr
The existing uintptr arithmetic is arguably valid because the
environment block is not located within the Go heap
(see golang/go#58625).

However, unsafe.Add (added in Go 1.17) expresses the same logic with
fewer conversions, and in addition avoids triggering the unsafeptr
vet check.

For golang/go#41205.

Change-Id: Ifc509279a13fd707be570908ec779d8518b4f75b
Reviewed-on: https://go-review.googlesource.com/c/sys/+/492415
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
2023-05-03 21:21:24 +00:00
Bryan C. Mills c79a742fd1 unix: fix a use-after-free bug in PtraceIO on freebsd
In CL 419915, both pointer fields of the PtraceIoDesc struct were
converted to type uintptr to address golang/go#54113.

However, that change was overzealous: the fix needed to convert fields
that refer to addresses in the child process, but the Addr field of
PtraceIoDesc is not even in the child process! It is instead an
address in the parent (Go) process.

Go's unsafe.Pointer rules prohibit converting a Go pointer to a
uintptr except when immediately converting back to an unsafe.Pointer
or calling a system call. Populating a PtraceIoDesc struct is neither
of those things, so converting the Addr field to uintptr introduced a
use-after-free bug.

This change reverts the change to the Addr field from CL 419915 and
consolidates the implementation of PtraceIO to reduce the the amount
of code that varies with GOARCH.

This change does not address the remaining ptrace uintptr bug
(golang/go#58387), which is also present in the Linux implementation.

Fixes golang/go#58351.
Updates golang/go#54113.
For golang/go#41205.

Change-Id: I14bdb4af42130aa7b4375e3f53fd1a0435f14307
Reviewed-on: https://go-review.googlesource.com/c/sys/+/465676
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2023-02-08 01:11:03 +00:00
Bryan C. Mills 90c8f94a05 unix: avoid converting non-pointers to unsafe.Pointer in PtraceIO
Despite having the misleading type "void*" in the C API, the "offs"
field of the ptrace_io_desc struct is an offset within the child
process, and thus is not necessarily a valid pointer at all in the
parent process. The Go unsafe.Pointer type must refer only to valid
pointers, so converting this field through unsafe.Pointer is incorrect
and (in some cases) dangerous.

While we're here, let's also rename the "addr" function argument to
"offs", since that's the corresponding ptrace_io_desc field. It's very
confusing to have a function argument named "attr" that doesn't map to
the struct field of the same name!

For golang/go#58351.

Change-Id: Id899f823e8d398b51fb0c42f466d7ae2f957c26b
Reviewed-on: https://go-review.googlesource.com/c/sys/+/465675
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2023-02-07 00:05:19 +00:00
Bryan C. Mills 4e121b1efb unix: add missing address operator in initxattrdest
The purpose of the _zero variable is to provide a valid address for a
pointer to an array of length zero. All other uses of the variable
take its address, but one reference to it (added in CL 147850043)
converts the variable (which has type uintptr) directly to an
unsafe.Pointer, producing a nil pointer instead of a non-nil pointer
to a zero-length array.

This typo is caught by 'go vet', but was masked for a long time by the
numerous false-positive warnings for the same check (#41205).

For golang/go#41205.

Change-Id: Ib3bebfadc6fc5574db19630169ff3f65da857bdd
Reviewed-on: https://go-review.googlesource.com/c/sys/+/465597
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
2023-02-06 18:23:25 +00:00
Bryan C. Mills 68f9dcb1f9 windows/debug/svc: buffer channel passed to signal.Notify
The os/signal package drops signal notifications if the channel is not
ready to receive immediately. To avoid dropping signals (for example,
while processing the other branch of a 'select'), the channel must be
buffered.

This fixes a mistake flagged by 'go vet ./...'.

Change-Id: I2a20dbe2aa27ae8ec009fff5e7be47e4409fdddd
Reviewed-on: https://go-review.googlesource.com/c/sys/+/465595
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
2023-02-06 18:23:23 +00:00
Bryan C. Mills 988cb79eb6 execabs: add legacy "//+build" constraints
Go releases before Go 1.17 are no longer supported, and Go 1.17
supports the newer "//go:build" syntax, so the older "//+build"
variation is no longer required by Go's support policy. However, the
x/sys module is very low-level and empirically some users are still on
unsupported Go releases, so there is little harm in adding the older
variations.

Fixes golang/go#52668.

Change-Id: Ieb8e64e450b9d795f2753fc5f99b36816a3dbff3
Reviewed-on: https://go-review.googlesource.com/c/sys/+/403695
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
2022-05-03 16:30:25 +00:00
Bryan C. Mills da31bd327a unix: lock the thread for TestPrctlRetInt
The prctl system calls used in this test modify and examine the
properties of the current thread. For the test to work correctly (and
to avoid interfering with other tests), the thread must be locked for
those calls and should not be reused when the test completes.

Fixes golang/go#50607

Change-Id: I2b14e6c0440155f4287d97f0bb72099af291ba0d
Reviewed-on: https://go-review.googlesource.com/c/sys/+/378555
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-01-14 19:58:35 +00:00
Bryan C. Mills dee7805ff2 windows: skip TestWinVerifyTrust
This test is failing (with different failure modes) on two different
builders. Adding a skip until it can be made more robust.

For golang/go#49651
For golang/go#49266
Updates golang/go#46906

Change-Id: I0fdd0e6f729c37e234b62b65abc53003eb8834f0
Reviewed-on: https://go-review.googlesource.com/c/sys/+/364794
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Patrik Nyblom <pnyb@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Patrik Nyblom <pnyb@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2021-11-17 18:06:35 +00:00
Bryan C. Mills c6105e7cf7 unix: in TestClockNanosleep, compare wall times when sleeping until wall times
Previously, we were sleeping until a wall time but then comparing the
duration slept using monotonic time. If the wall time happens to jump
during the sleep, the monotonic time can legitimately be shorter than
the wall time.

Fixes golang/go#42513

Change-Id: Iaef8d9ba46082e821f5cb18c96b9869238d7af05
Reviewed-on: https://go-review.googlesource.com/c/sys/+/363456
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2021-11-12 14:30:42 +00:00
Bryan C. Mills f221eed1c0 unix: in TestSelect, only error for an early wakeup on Linux
On Linux, 'man 2 select' explicitly lists the conditions under which
select may return before the timeout interval. Most other platforms
make no such guarantee, so do not test for it on those platforms.

Fixes golang/go#36409

Change-Id: I194a34af3132a7db7fc186dec9de67fa6dd0bfae
Reviewed-on: https://go-review.googlesource.com/c/sys/+/363455
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-11-11 21:35:25 +00:00
Bryan C. Mills 99a53858aa all: add missing //go:build comments
These were apparently overlooked in CL 357329, CL 294490, CL 296889,
and other various updates to this module. (I noticed them via gopls
while investigating golang/go#49466.)

Updates golang/go#41184

Change-Id: Id042bb6fe5282e6d528e9315acf2ad29d0df58ba
Reviewed-on: https://go-review.googlesource.com/c/sys/+/362577
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-11-10 15:43:04 +00:00
Bryan C. Mills 51b60fd695 windows/registry: lock OS thread while enumerating keys
CL 361154 updated the standard syscall package to document that
successive calls to syscall.RegEnumKeyEx must occur on the same OS
thread (after that requirement was empirically discovered in
golang/go#49320).

This use in x/sys needs to be updated to comply with the
newly-discovered requirement.

Fixes golang/go#49466.

Change-Id: Idc45d91f175e1db25c215213fcaa45982c2f5e6e
Reviewed-on: https://go-review.googlesource.com/c/sys/+/362576
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-11-09 18:48:56 +00:00
Bryan C. Mills 0981d6026f go.mod: upgrade to go 1.17
This change was produced using 'go mod tidy -go=1.17'
with a go command built at CL 315210.

This activates lazy loading, and updates the go.mod file to maintain
the lazy-loading invariants (namely, including an explicit requirement
for every package transitively imported by the main module).

Note that this does *not* prevent users with earlier go versions from
successfully building packages from this module.

(This has little to no effect for the sys module today because it does
not itself have any module dependencies.)

For golang/go#36460.

Change-Id: I0d278e13e54f961a42cd890ee248ddef811f1a1c
Reviewed-on: https://go-review.googlesource.com/c/sys/+/316111
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2021-05-03 17:37:54 +00:00
Bryan C. Mills 57ea560cfc windows: comment on fields omitted from the win32finddata1 struct
Updates golang/go#42637

Change-Id: I2e8c95af3cf2172c55bda6a15a749afcc6c45581
Reviewed-on: https://go-review.googlesource.com/c/sys/+/284172
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
2021-02-19 17:28:41 +00:00
Bryan C. Mills d923437fa5 unix: use a type equivalent to reflect.SliceHeader to manipulate slices
The regression test included in this change verifies that the type is,
in fact, equivalent, while allowing the actual header definitions to
avoid importing the (relatively heavy) "reflect" package itself.

This change is loosely based on Keyan Pishdadian's draft in CL 230557.

For golang/go#37805

Change-Id: I998c69cdeb852154cd66ab5fdaa542a6f19666a2
Reviewed-on: https://go-review.googlesource.com/c/sys/+/231177
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
2020-04-30 20:27:03 +00:00
Bryan C. Mills b294cbcfc5 all: add go directive to go.mod
Go 1.12 adds this line automatically, so let's just go with it.

Change-Id: I0ac156a58f0f107e019a8c06ebf189b819a3651d
Reviewed-on: https://go-review.googlesource.com/c/sys/+/165751
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-03-06 22:07:23 +00:00
Bryan C. Mills b354f8bf4d cpu: define cacheLineSize and doinit for WASM
This fixes a build break when compiling this package with
GOOS=js / GOARCH=wasm.

Updates golang/go#30241

Change-Id: I0b35aa50a69370ed0d83c522e12ce7613c4c3ec4
Reviewed-on: https://go-review.googlesource.com/c/sys/+/165749
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-03-06 22:02:34 +00:00
Bryan C. Mills 629670e5ac windows: gofmt -w windows/security_windows.go
x/sys/unix is vendored into the standard library, and the commit hook
for the standard library requires files to be gofmt-clean.

Updates golang/go#26924

Change-Id: I22a994062bcdbebe8a1fe1ae0ed4606837f03079
Reviewed-on: https://go-review.googlesource.com/c/162990
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-02-20 15:41:26 +00:00
Bryan C. Mills 90b0e4468f unix: do not invoke Mkfifo with a relative path in a read-only directory
Updates golang/go#28387

Change-Id: Ibcdc3f9cb3dc43b86b7e7d3539ed592219e54aba
Reviewed-on: https://go-review.googlesource.com/c/162987
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-02-19 20:33:50 +00:00