In CL 421441, we changed syscall to allow concurrent calls to
forkExec.
On platforms that support the pipe2 syscall that is the right
behavior, because pipe2 atomically opens the pipe with CLOEXEC already
set.
However, on platforms that do not support pipe2 (currently aix and
darwin), syscall.forkExecPipe is not atomic, and the pipes do not
initially have CLOEXEC set. If two calls to forkExec proceed
concurrently, a pipe intended for one child process can be
accidentally inherited by the other. If the process is long-lived, the
pipe can be held open unexpectedly and prevent the parent process from
reaching EOF reading the child's status from the pipe.
Fixes#61080.
Updates #23558.
Updates #54162.
Change-Id: I83edcc80674ff267a39d06260c5697c654ff5a4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/507355
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
The workaround in CL 69970044 introduced a panic when StartProcess is
called with empty argv. Check the length before trying to access it.
Change-Id: Ic948d86c7067a21c484ba24e100d1f1f80179730
Reviewed-on: https://go-review.googlesource.com/c/go/+/500415
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
If we increased the NOFILE rlimit when starting the program,
restore the original rlimit when forking a child process.
For #46279
Change-Id: Ia5d2af9ef435e5932965c15eec2e428d2130d230
Reviewed-on: https://go-review.googlesource.com/c/go/+/476097
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.
Fixes#50057
Fixes CVE-2021-44717
Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370576
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
When these packages are released as part of Go 1.18,
Go 1.16 will no longer be supported, so we can remove
the +build tags in these files.
Ran go fix -fix=buildtag std cmd and then reverted the bootstrapDirs
as defined in src/cmd/dist/buildtool.go, which need to continue
to build with Go 1.4 for now.
Also reverted src/vendor and src/cmd/vendor, which will need
to be updated in their own repos first.
Manual changes in runtime/pprof/mprof_test.go to adjust line numbers.
For #41184.
Change-Id: Ic0f93f7091295b6abc76ed5cd6e6746e1280861e
Reviewed-on: https://go-review.googlesource.com/c/go/+/344955
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Convert the syscall package on openbsd/arm to use libc rather than performing
direct system calls.
Updates #36435
Change-Id: Iff3a91c959292cbf4e0a09c7fd34efc8e88ff83f
Reviewed-on: https://go-review.googlesource.com/c/go/+/315793
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Convert the syscall package on openbsd/386 to use libc rather than performing
direct system calls.
Updates #36435
Change-Id: Ifcfbca0e6b933762596a564243caa850dac01442
Reviewed-on: https://go-review.googlesource.com/c/go/+/287654
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Like on openbsd/amd64, use libc instead of direct syscalls on
openbsd/arm64.
This was likely missed in CL 286815. A similar change was done for
openbsd/amd64 in CL 270380.
Updates #36435
Change-Id: Ie496a6130f1a43d30974502777db12217e65c551
Reviewed-on: https://go-review.googlesource.com/c/go/+/312229
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Make all our package sources use Go 1.17 gofmt format
(adding //go:build lines).
Part of //go:build change (#41184).
See https://golang.org/design/draft-gobuild
Change-Id: Ia0534360e4957e58cd9a18429c39d0e32a6addb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/294430
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Convert the syscall package on openbsd/amd64 to use libc rather than performing
direct system calls.
Updates #36435
Change-Id: Ieb5926a91ed34f7c722e3667004ec484c86804ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/270380
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Introduce GOOS=ios for iOS systems. GOOS=ios matches "darwin"
build tag, like GOOS=android matches "linux" and GOOS=illumos
matches "solaris". Only ios/arm64 is supported (ios/amd64 is
not).
GOOS=ios and GOOS=darwin remain essentially the same at this
point. They will diverge at later time, to differentiate macOS
and iOS.
Uses of GOOS=="darwin" are changed to (GOOS=="darwin" || GOOS=="ios"),
except if it clearly means macOS (e.g. GOOS=="darwin" && GOARCH=="amd64"),
it remains GOOS=="darwin".
Updates #38485.
Change-Id: I4faacdc1008f42434599efb3c3ad90763a83b67c
Reviewed-on: https://go-review.googlesource.com/c/go/+/254740
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Historically we've assumed that we can install all signal handlers
with the SA_RESTART flag set, and let the system restart slow functions
if a signal is received. Therefore, we don't have to worry about EINTR.
This is only partially true, and we've added EINTR checks already for
connect, and open/read on Darwin, and sendfile on Solaris.
Other cases have turned up in #36644, #38033, and #38836.
Also, #20400 points out that when Go code is included in a C program,
the C program may install its own signal handlers without SA_RESTART.
In that case, Go code will see EINTR no matter what it does.
So, go ahead and check for EINTR. We don't check in the syscall package;
people using syscalls directly may want to check for EINTR themselves.
But we do check for EINTR in the higher level APIs in os and net,
and retry the system call if we see it.
This change looks safe, but of course we may be missing some cases
where we need to check for EINTR. As such cases turn up, we can add
tests to runtime/testdata/testprogcgo/eintr.go, and fix the code.
If there are any such cases, their handling after this change will be
no worse than it is today.
For #22838Fixes#20400Fixes#36644Fixes#38033Fixes#38836
Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896
Reviewed-on: https://go-review.googlesource.com/c/go/+/232862
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ctty was always handled as a child descriptor, but in some cases
passing a parent descriptor would also work. This depended on
unpredictable details of the implementation. Reject those cases to
avoid confusion.
Also reject setting both Setctty and Foreground, as they use Ctty
in incompatible ways. It's unlikely that any programs set both fields,
as they don't make sense together.
Fixes#29458
Change-Id: Ieba2d625711fd4b82c8e65e1feed02fd1fb25e6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231638
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Like GOOS=android which implies the "linux" build tag, GOOS=illumos
implies the "solaris" build tag. This lets the existing ecosystem of
packages still work on illumos, but still permits packages to start
differentiating between solaris and illumos.
Fixes#20603
Change-Id: I8f4eabf1a66060538dca15d7658c1fbc6c826622
Reviewed-on: https://go-review.googlesource.com/c/go/+/174457
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of allocating a byte slice for every string,
calculated the required size and create
a single slice big enough to hold all of them.
As an added benefit, any error encountered
will now be returned before allocations occur.
os/exec package benchmarks:
name old time/op new time/op delta
ExecEcho-8 2.14ms ± 1% 2.14ms ± 3% ~ (p=0.842 n=10+9)
name old alloc/op new alloc/op delta
ExecEcho-8 6.35kB ± 0% 6.18kB ± 0% -2.65% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
ExecEcho-8 69.0 ± 0% 36.0 ± 0% -47.83% (p=0.000 n=10+10)
Change-Id: I84118d8473037d873f73903d4e4f6ed14f531ce7
Reviewed-on: https://go-review.googlesource.com/c/164961
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Miscellaneous additional conversions from raw syscalls
to using their libc equivalent.
Update #17490
Change-Id: If9ab22cc1d676c1f20fb161ebf02b0c28f71585d
Reviewed-on: https://go-review.googlesource.com/c/148257
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
AIX and Solaris both requires libc to make any syscalls and their
implementation is really similar.
Therefore, Solaris files reused by AIX have their name changed to *_libc.
exec_libc.go is also adapted to AIX.
Updates: #25893
Change-Id: I50d1d7b964831637013d5e64799187cd9565c42b
Reviewed-on: https://go-review.googlesource.com/c/138719
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The test added for issue #18146 exposed a long-existing bug in the
Solaris port; notably, that syscall.Exec uses RawSyscall -- which is not
actually functional for the Solaris port (intentionally) and only exists
as a placebo to satisfy build requirements.
Call syscall.execve instead for Solaris.
Fixes#20832
Change-Id: I327d863f4bbbbbb6e5ecf66b82152c4030825d09
Reviewed-on: https://go-review.googlesource.com/47032
Run-TryBot: Shawn Walker-Salas <shawn.walker@oracle.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Linux's execve has (at the time of writing, and since v2.6.30) a bug when it ran
concurrently with clone, in that it would fail to set up some datastructures if
the thread count before and after some steps differed. This is described better
and in more detail by Colin King in Launchpad¹ and kernel² bugs. When a program
written in Go runtime.Exec's a setuid binary, this issue may cause the resulting
process to not have the expected uid. This patch works around the issue by using
a mutex to serialize exec and clone.
1. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1672819
2. https://bugzilla.kernel.org/show_bug.cgi?id=195453Fixes#19546
Change-Id: I126e87d1d9ce3be5ea4ec9c7ffe13f92e087903d
Reviewed-on: https://go-review.googlesource.com/43713
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If the caller set ups a Credential in os/exec.Command,
os/exec.Command.Start will end up calling setgroups(2), even if no
supplementary groups were given.
Only root can call setgroups(2) on BSD kernels, which causes Start to
fail for non-root users when they try to set uid and gid for the new
process.
We fix by introducing a new field to syscall.Credential named
NoSetGroups, and setgroups(2) is only called if it is false.
We make this field with inverted logic to preserve backward
compatibility.
RELNOTES=yes
Change-Id: I3cff1f21c117a1430834f640ef21fd4e87e06804
Reviewed-on: https://go-review.googlesource.com/36697
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is a change improving consistency in the source tree.
The pattern foo &= ^bar, was only used six times in src/ directory.
The usage of the supported &^ (bit clear / AND NOT) operator is way more
common, about factor 10x.
Change-Id: If26a2994fd81d23d42189bee00245eb84e672cf3
Reviewed-on: https://go-review.googlesource.com/21224
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Bryan C. Mills
Roland Shoemaker
Ian Lance Taylor
Russ Cox
Joel Sing
Tobias Klauser
Cherry Zhang
Joshua M. Clulow
Josh Bleecher Snyder
Keith Randall
Clément Chigot
Shawn Walker-Salas
John R. Lenton
Wander Lairson Costa
Kevin Burke
Marvin Stenger
Brad Fitzpatrick
Shenghou Ma