From 0e7138a102803f04254ea56c0bc9ba89007ffbe5 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 21 Jul 2021 11:43:57 -0700 Subject: [release-branch.go1.17] runtime: mark TestGcSys as flaky I don't know what this test is doing, but it very frequently flakes for me while testing mundane compiler CLs. According to the issue log, it's been flaky for ~3 years. Updates #37331. Fixes #52826. Change-Id: I81c43ad646ee12d4c6561290a54e4bf637695bc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/336349 Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Michael Knyszek (cherry picked from commit d8ceb133cac65b47c3f5bb292fbb28690c8b89a5) Reviewed-on: https://go-review.googlesource.com/c/go/+/406974 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov --- src/runtime/gc_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/gc_test.go b/src/runtime/gc_test.go index 5e7c6c574f..0ec5331534 100644 --- a/src/runtime/gc_test.go +++ b/src/runtime/gc_test.go @@ -21,6 +21,7 @@ import ( ) func TestGcSys(t *testing.T) { + t.Skip("skipping known-flaky test; golang.org/issue/37331") if os.Getenv("GOGC") == "off" { t.Skip("skipping test; GOGC=off in environment") } -- cgit v1.2.3-54-g00ecf From a9003376d55bcbd5b2b24e199ca861058f84c94c Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 30 Sep 2021 15:15:45 -0400 Subject: [release-branch.go1.17] cmd/dist: consistently set PWD when executing a command in a different directory Fixes #52995 Updates #33598 Change-Id: If0de906ffa2fcc83bb2a90f9e80a5b29d7667398 Reviewed-on: https://go-review.googlesource.com/c/go/+/353449 Trust: Bryan C. Mills Reviewed-by: Ian Lance Taylor (cherry picked from commit c035d829e9fbd150148a1738020fe9c155cda61f) Reviewed-on: https://go-review.googlesource.com/c/go/+/407881 Reviewed-by: Heschi Kreinick Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot --- src/cmd/dist/exec.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/cmd/dist/test.go | 54 +++++++++++++++++++++++++--------------------------- src/cmd/dist/util.go | 2 +- 3 files changed, 80 insertions(+), 29 deletions(-) create mode 100644 src/cmd/dist/exec.go diff --git a/src/cmd/dist/exec.go b/src/cmd/dist/exec.go new file mode 100644 index 0000000000..67305530ae --- /dev/null +++ b/src/cmd/dist/exec.go @@ -0,0 +1,53 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "os" + "os/exec" + "strings" +) + +// setDir sets cmd.Dir to dir, and also adds PWD=dir to cmd's environment. +func setDir(cmd *exec.Cmd, dir string) { + cmd.Dir = dir + setEnv(cmd, "PWD", dir) +} + +// setEnv sets cmd.Env so that key = value. +// +// It first removes any existing values for key, so it is safe to call +// even from within cmdbootstrap. +func setEnv(cmd *exec.Cmd, key, value string) { + kv := key + "=" + value + if cmd.Env == nil { + cmd.Env = os.Environ() + } + + prefix := kv[:len(key)+1] + for i, entry := range cmd.Env { + if strings.HasPrefix(entry, prefix) { + cmd.Env[i] = kv + return + } + } + + cmd.Env = append(cmd.Env, kv) +} + +// unsetEnv sets cmd.Env so that key is not present in the environment. +func unsetEnv(cmd *exec.Cmd, key string) { + if cmd.Env == nil { + cmd.Env = os.Environ() + } + + prefix := key + "=" + for i, entry := range cmd.Env { + if strings.HasPrefix(entry, prefix) { + cmd.Env = append(cmd.Env[:i], cmd.Env[i+1:]...) + return + } + } +} diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index f40fa926df..beb7c4650c 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -522,7 +522,8 @@ func (t *tester) registerTests() { heading: "GOOS=ios on darwin/amd64", fn: func(dt *distTest) error { cmd := t.addCmd(dt, "src", t.goTest(), t.timeout(300), "-run=SystemRoots", "crypto/x509") - cmd.Env = append(os.Environ(), "GOOS=ios", "CGO_ENABLED=1") + setEnv(cmd, "GOOS", "ios") + setEnv(cmd, "CGO_ENABLED", "1") return nil }, }) @@ -542,7 +543,7 @@ func (t *tester) registerTests() { cmd := t.addCmd(dt, "src", t.goTest(), t.timeout(300), "runtime", "-cpu=1,2,4", "-quick") // We set GOMAXPROCS=2 in addition to -cpu=1,2,4 in order to test runtime bootstrap code, // creation of first goroutines and first garbage collections in the parallel setting. - cmd.Env = append(os.Environ(), "GOMAXPROCS=2") + setEnv(cmd, "GOMAXPROCS", "2") return nil }, }) @@ -563,7 +564,7 @@ func (t *tester) registerTests() { return nil } cmd := exec.Command("go", "test") - cmd.Dir = filepath.Join(os.Getenv("GOROOT"), "src/cmd/go/testdata/testterminal18153") + setDir(cmd, filepath.Join(os.Getenv("GOROOT"), "src/cmd/go/testdata/testterminal18153")) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr return cmd.Run() @@ -600,16 +601,13 @@ func (t *tester) registerTests() { return err } - // Run `go test fmt` in the moved GOROOT. + // Run `go test fmt` in the moved GOROOT, without explicitly setting + // GOROOT in the environment. The 'go' command should find itself. cmd := exec.Command(filepath.Join(moved, "bin", "go"), "test", "fmt") cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - // Don't set GOROOT in the environment. - for _, e := range os.Environ() { - if !strings.HasPrefix(e, "GOROOT=") && !strings.HasPrefix(e, "GOCACHE=") { - cmd.Env = append(cmd.Env, e) - } - } + unsetEnv(cmd, "GOROOT") + unsetEnv(cmd, "GOCACHE") // TODO(bcmills): ...why‽ err := cmd.Run() if rerr := os.Rename(moved, goroot); rerr != nil { @@ -736,11 +734,9 @@ func (t *tester) registerTests() { heading: "../misc/swig/callback", fn: func(dt *distTest) error { cmd := t.addCmd(dt, "misc/swig/callback", t.goTest()) - cmd.Env = append(os.Environ(), - "CGO_CFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", - "CGO_CXXFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", - "CGO_LDFLAGS=-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option", - ) + setEnv(cmd, "CGO_CFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option") + setEnv(cmd, "CGO_CXXFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option") + setEnv(cmd, "CGO_LDFLAGS", "-flto -Wno-lto-type-mismatch -Wno-unknown-warning-option") return nil }, }, @@ -892,9 +888,9 @@ func (t *tester) registerSeqTest(name, dirBanner string, cmdline ...interface{}) func (t *tester) bgDirCmd(dir, bin string, args ...string) *exec.Cmd { cmd := exec.Command(bin, args...) if filepath.IsAbs(dir) { - cmd.Dir = dir + setDir(cmd, dir) } else { - cmd.Dir = filepath.Join(goroot, dir) + setDir(cmd, filepath.Join(goroot, dir)) } return cmd } @@ -1132,7 +1128,8 @@ func (t *tester) runHostTest(dir, pkg string) error { defer os.Remove(f.Name()) cmd := t.dirCmd(dir, t.goTest(), "-c", "-o", f.Name(), pkg) - cmd.Env = append(os.Environ(), "GOARCH="+gohostarch, "GOOS="+gohostos) + setEnv(cmd, "GOARCH", gohostarch) + setEnv(cmd, "GOOS", gohostos) if err := cmd.Run(); err != nil { return err } @@ -1141,7 +1138,7 @@ func (t *tester) runHostTest(dir, pkg string) error { func (t *tester) cgoTest(dt *distTest) error { cmd := t.addCmd(dt, "misc/cgo/test", t.goTest()) - cmd.Env = append(os.Environ(), "GOFLAGS=-ldflags=-linkmode=auto") + setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=auto") // Skip internal linking cases on linux/arm64 to support GCC-9.4 and above. // See issue #39466. @@ -1149,7 +1146,7 @@ func (t *tester) cgoTest(dt *distTest) error { if t.internalLink() && !skipInternalLink { cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-tags=internal") - cmd.Env = append(os.Environ(), "GOFLAGS=-ldflags=-linkmode=internal") + setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=internal") } pair := gohostos + "-" + goarch @@ -1161,9 +1158,9 @@ func (t *tester) cgoTest(dt *distTest) error { break } cmd := t.addCmd(dt, "misc/cgo/test", t.goTest()) - cmd.Env = append(os.Environ(), "GOFLAGS=-ldflags=-linkmode=external") + setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=external") - cmd = t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags", "-linkmode=external -s") + t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags", "-linkmode=external -s") if t.supportedBuildmode("pie") { t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie") @@ -1181,10 +1178,10 @@ func (t *tester) cgoTest(dt *distTest) error { "openbsd-386", "openbsd-amd64", "openbsd-arm", "openbsd-arm64", "openbsd-mips64": cmd := t.addCmd(dt, "misc/cgo/test", t.goTest()) - cmd.Env = append(os.Environ(), "GOFLAGS=-ldflags=-linkmode=external") + setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=external") // cgo should be able to cope with both -g arguments and colored // diagnostics. - cmd.Env = append(cmd.Env, "CGO_CFLAGS=-g0 -fdiagnostics-color") + setEnv(cmd, "CGO_CFLAGS", "-g0 -fdiagnostics-color") t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-ldflags", "-linkmode=auto") t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-ldflags", "-linkmode=external") @@ -1217,7 +1214,7 @@ func (t *tester) cgoTest(dt *distTest) error { // than -static in -extldflags, so test both. // See issue #16651. cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-tags=static") - cmd.Env = append(os.Environ(), "CGO_LDFLAGS=-static -pthread") + setEnv(cmd, "CGO_LDFLAGS", "-static -pthread") } } @@ -1456,7 +1453,7 @@ func (t *tester) raceTest(dt *distTest) error { // We shouldn't need to redo all of misc/cgo/test too. // The race buildler will take care of this. // cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-race") - // cmd.Env = append(os.Environ(), "GOTRACEBACK=2") + // setEnv(cmd, "GOTRACEBACK", "2") } if t.extLink() { // Test with external linking; see issue 9133. @@ -1486,7 +1483,8 @@ func (t *tester) testDirTest(dt *distTest, shard, shards int) error { }) cmd := t.dirCmd("test", "go", "build", "-o", runtest.exe, "run.go") - cmd.Env = append(os.Environ(), "GOOS="+gohostos, "GOARCH="+gohostarch) + setEnv(cmd, "GOOS", gohostos) + setEnv(cmd, "GOARCH", gohostarch) runtest.err = cmd.Run() }) if runtest.err != nil { @@ -1650,7 +1648,7 @@ func (t *tester) runPrecompiledStdTest(timeout time.Duration) error { bin := t.prebuiltGoPackageTestBinary() fmt.Fprintf(os.Stderr, "# %s: using pre-built %s...\n", stdMatches[0], bin) cmd := exec.Command(bin, "-test.short="+short(), "-test.timeout="+timeout.String()) - cmd.Dir = filepath.Dir(bin) + setDir(cmd, filepath.Dir(bin)) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Start(); err != nil { diff --git a/src/cmd/dist/util.go b/src/cmd/dist/util.go index df60145d1e..28fe5e1d8d 100644 --- a/src/cmd/dist/util.go +++ b/src/cmd/dist/util.go @@ -72,7 +72,7 @@ func run(dir string, mode int, cmd ...string) string { } xcmd := exec.Command(cmd[0], cmd[1:]...) - xcmd.Dir = dir + setDir(xcmd, dir) var data []byte var err error -- cgit v1.2.3-54-g00ecf From e846f3f2d602ec5fd4689e14d7530d894b807e70 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 8 Feb 2022 15:07:21 -0500 Subject: [release-branch.go1.17] runtime: skip TestGdbBacktrace flakes matching a known GDB internal error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestGdbBacktrace occasionally fails due to a GDB internal error. We have observed the error on various linux builders since at least October 2020, and it has been reported upstream at least twice.¹² Since the bug is external to the Go project and does not appear to be fixed upstream, this failure mode can only add noise. ¹https://sourceware.org/bugzilla/show_bug.cgi?id=24628 ²https://sourceware.org/bugzilla/show_bug.cgi?id=28551 Fixes #53049 Updates #43068 Change-Id: I6c92006a5d730f1c4df54b0307f080b3d643cc6b Reviewed-on: https://go-review.googlesource.com/c/go/+/384234 Trust: Bryan Mills Run-TryBot: Bryan Mills Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot (cherry picked from commit 275aedccd4f2beae82dbf96c94a6c1c9b365a647) Reviewed-on: https://go-review.googlesource.com/c/go/+/408054 Reviewed-by: Alex Rakoczy --- src/runtime/runtime-gdb_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/runtime/runtime-gdb_test.go b/src/runtime/runtime-gdb_test.go index 8c76a9123c..097d1b5c6a 100644 --- a/src/runtime/runtime-gdb_test.go +++ b/src/runtime/runtime-gdb_test.go @@ -427,6 +427,9 @@ func TestGdbBacktrace(t *testing.T) { got, err := exec.Command("gdb", args...).CombinedOutput() t.Logf("gdb output:\n%s", got) if err != nil { + if bytes.Contains(got, []byte("internal-error: wait returned unexpected status 0x0")) { + testenv.SkipFlaky(t, 43068) + } t.Fatalf("gdb exited with error: %v", err) } -- cgit v1.2.3-54-g00ecf From 65701ad2b430466dd4bd6e1df107f81c0f8ee9cb Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Mon, 23 May 2022 20:20:07 -0400 Subject: [release-branch.go1.17] misc/cgo/testsanitizers: use buffered channel in tsan12.go os/signal.Notify requires that "the caller must ensure that c has sufficient buffer space to keep up with the expected signal rate" as it does a nonblocking send when it receives a signal. The test currently using a unbuffered channel, which means it may miss the signal if the signal arrives before the channel receive operation. Fixes #53042. Updates #52998. Change-Id: Icdcab9396d735506480ef880fb45a4669fa7cc8f Reviewed-on: https://go-review.googlesource.com/c/go/+/407888 Reviewed-by: Ian Lance Taylor Reviewed-by: Bryan Mills Run-TryBot: Cherry Mui Reviewed-by: Cuong Manh Le TryBot-Result: Gopher Robot (cherry picked from commit 62e130226767a088ace196da90a774c9a9d14689) Reviewed-on: https://go-review.googlesource.com/c/go/+/408115 --- misc/cgo/testsanitizers/testdata/tsan12.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/cgo/testsanitizers/testdata/tsan12.go b/misc/cgo/testsanitizers/testdata/tsan12.go index 3e767eee1f..0ef545d09b 100644 --- a/misc/cgo/testsanitizers/testdata/tsan12.go +++ b/misc/cgo/testsanitizers/testdata/tsan12.go @@ -22,7 +22,7 @@ import ( import "C" func main() { - ch := make(chan os.Signal) + ch := make(chan os.Signal, 1) signal.Notify(ch, syscall.SIGUSR1) if err := exec.Command("true").Run(); err != nil { -- cgit v1.2.3-54-g00ecf From 2be03d789de905a4b050ff5f3a51b724e1b09494 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 25 Apr 2022 19:02:35 -0700 Subject: [release-branch.go1.17] crypto/rand: properly handle large Read on windows Use the batched reader to chunk large Read calls on windows to a max of 1 << 31 - 1 bytes. This prevents an infinite loop when trying to read more than 1 << 32 -1 bytes, due to how RtlGenRandom works. This change moves the batched function from rand_unix.go to rand.go, since it is now needed for both windows and unix implementations. Updates #52561 Fixes #52932 Fixes CVE-2022-30634 Change-Id: Id98fc4b1427e5cb2132762a445b2aed646a37473 Reviewed-on: https://go-review.googlesource.com/c/go/+/402257 Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda Reviewed-by: Filippo Valsorda TryBot-Result: Gopher Robot (cherry picked from commit bb1f4416180511231de6d17a1f2f55c82aafc863) Reviewed-on: https://go-review.googlesource.com/c/go/+/406635 Reviewed-by: Damien Neil --- src/crypto/rand/rand.go | 18 ++++++++++++++++++ src/crypto/rand/rand_batched.go | 22 ++++++---------------- src/crypto/rand/rand_batched_test.go | 21 +++++++++++---------- src/crypto/rand/rand_getentropy.go | 6 +++--- src/crypto/rand/rand_unix.go | 4 ++-- src/crypto/rand/rand_windows.go | 18 ++++++------------ 6 files changed, 46 insertions(+), 43 deletions(-) diff --git a/src/crypto/rand/rand.go b/src/crypto/rand/rand.go index fddd1147e6..f2c276008d 100644 --- a/src/crypto/rand/rand.go +++ b/src/crypto/rand/rand.go @@ -23,3 +23,21 @@ var Reader io.Reader func Read(b []byte) (n int, err error) { return io.ReadFull(Reader, b) } + +// batched returns a function that calls f to populate a []byte by chunking it +// into subslices of, at most, readMax bytes. +func batched(f func([]byte) error, readMax int) func([]byte) error { + return func(out []byte) error { + for len(out) > 0 { + read := len(out) + if read > readMax { + read = readMax + } + if err := f(out[:read]); err != nil { + return err + } + out = out[read:] + } + return nil + } +} diff --git a/src/crypto/rand/rand_batched.go b/src/crypto/rand/rand_batched.go index d7c5bf3562..8df715fdd1 100644 --- a/src/crypto/rand/rand_batched.go +++ b/src/crypto/rand/rand_batched.go @@ -8,6 +8,7 @@ package rand import ( + "errors" "internal/syscall/unix" ) @@ -16,20 +17,6 @@ func init() { altGetRandom = batched(getRandomBatch, maxGetRandomRead) } -// batched returns a function that calls f to populate a []byte by chunking it -// into subslices of, at most, readMax bytes. -func batched(f func([]byte) bool, readMax int) func([]byte) bool { - return func(buf []byte) bool { - for len(buf) > readMax { - if !f(buf[:readMax]) { - return false - } - buf = buf[readMax:] - } - return len(buf) == 0 || f(buf) - } -} - // If the kernel is too old to support the getrandom syscall(), // unix.GetRandom will immediately return ENOSYS and we will then fall back to // reading from /dev/urandom in rand_unix.go. unix.GetRandom caches the ENOSYS @@ -37,7 +24,10 @@ func batched(f func([]byte) bool, readMax int) func([]byte) bool { // If the kernel supports the getrandom() syscall, unix.GetRandom will block // until the kernel has sufficient randomness (as we don't use GRND_NONBLOCK). // In this case, unix.GetRandom will not return an error. -func getRandomBatch(p []byte) (ok bool) { +func getRandomBatch(p []byte) error { n, err := unix.GetRandom(p, 0) - return n == len(p) && err == nil + if n != len(p) { + return errors.New("short read") + } + return err } diff --git a/src/crypto/rand/rand_batched_test.go b/src/crypto/rand/rand_batched_test.go index 2d20922c82..b56345e50f 100644 --- a/src/crypto/rand/rand_batched_test.go +++ b/src/crypto/rand/rand_batched_test.go @@ -9,20 +9,21 @@ package rand import ( "bytes" + "errors" "testing" ) func TestBatched(t *testing.T) { - fillBatched := batched(func(p []byte) bool { + fillBatched := batched(func(p []byte) error { for i := range p { p[i] = byte(i) } - return true + return nil }, 5) p := make([]byte, 13) - if !fillBatched(p) { - t.Fatal("batched function returned false") + if err := fillBatched(p); err != nil { + t.Fatalf("batched function returned error: %s", err) } expected := []byte{0, 1, 2, 3, 4, 0, 1, 2, 3, 4, 0, 1, 2} if !bytes.Equal(expected, p) { @@ -31,15 +32,15 @@ func TestBatched(t *testing.T) { } func TestBatchedError(t *testing.T) { - b := batched(func(p []byte) bool { return false }, 5) - if b(make([]byte, 13)) { - t.Fatal("batched function should have returned false") + b := batched(func(p []byte) error { return errors.New("") }, 5) + if b(make([]byte, 13)) == nil { + t.Fatal("batched function should have returned an error") } } func TestBatchedEmpty(t *testing.T) { - b := batched(func(p []byte) bool { return false }, 5) - if !b(make([]byte, 0)) { - t.Fatal("empty slice should always return true") + b := batched(func(p []byte) error { return errors.New("") }, 5) + if err := b(make([]byte, 0)); err != nil { + t.Fatalf("empty slice should always return nil: %s", err) } } diff --git a/src/crypto/rand/rand_getentropy.go b/src/crypto/rand/rand_getentropy.go index dd725372ad..b1c19f3d0d 100644 --- a/src/crypto/rand/rand_getentropy.go +++ b/src/crypto/rand/rand_getentropy.go @@ -15,7 +15,7 @@ func init() { altGetRandom = getEntropy } -func getEntropy(p []byte) (ok bool) { +func getEntropy(p []byte) error { // getentropy(2) returns a maximum of 256 bytes per call for i := 0; i < len(p); i += 256 { end := i + 256 @@ -24,8 +24,8 @@ func getEntropy(p []byte) (ok bool) { } err := unix.GetEntropy(p[i:end]) if err != nil { - return false + return err } } - return true + return nil } diff --git a/src/crypto/rand/rand_unix.go b/src/crypto/rand/rand_unix.go index 81277eb6a5..3d11159a34 100644 --- a/src/crypto/rand/rand_unix.go +++ b/src/crypto/rand/rand_unix.go @@ -46,7 +46,7 @@ type devReader struct { // altGetRandom if non-nil specifies an OS-specific function to get // urandom-style randomness. -var altGetRandom func([]byte) (ok bool) +var altGetRandom func([]byte) (err error) func warnBlocked() { println("crypto/rand: blocked for 60 seconds waiting to read random data from the kernel") @@ -59,7 +59,7 @@ func (r *devReader) Read(b []byte) (n int, err error) { t := time.AfterFunc(60*time.Second, warnBlocked) defer t.Stop() } - if altGetRandom != nil && r.name == urandomDevice && altGetRandom(b) { + if altGetRandom != nil && r.name == urandomDevice && altGetRandom(b) == nil { return len(b), nil } r.mu.Lock() diff --git a/src/crypto/rand/rand_windows.go b/src/crypto/rand/rand_windows.go index 7379f1489a..6c0655c72b 100644 --- a/src/crypto/rand/rand_windows.go +++ b/src/crypto/rand/rand_windows.go @@ -9,7 +9,6 @@ package rand import ( "internal/syscall/windows" - "os" ) func init() { Reader = &rngReader{} } @@ -17,16 +16,11 @@ func init() { Reader = &rngReader{} } type rngReader struct{} func (r *rngReader) Read(b []byte) (n int, err error) { - // RtlGenRandom only accepts 2**32-1 bytes at a time, so truncate. - inputLen := uint32(len(b)) - - if inputLen == 0 { - return 0, nil - } - - err = windows.RtlGenRandom(b) - if err != nil { - return 0, os.NewSyscallError("RtlGenRandom", err) + // RtlGenRandom only returns 1<<32-1 bytes at a time. We only read at + // most 1<<31-1 bytes at a time so that this works the same on 32-bit + // and 64-bit systems. + if err := batched(windows.RtlGenRandom, 1<<31-1)(b); err != nil { + return 0, err } - return int(inputLen), nil + return len(b), nil } -- cgit v1.2.3-54-g00ecf From 590b53fac9ebdb259b32e82805dec1cc96987930 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 3 May 2022 15:14:56 -0400 Subject: [release-branch.go1.17] os/exec: return clear error for missing cmd.Path Following up on CL 403694, there is a bit of confusion about when Path is and isn't set, along with now the exported Err field. Catch the case where Path and Err (and lookPathErr) are all unset and give a helpful error. Updates #52574 Followup after #43724. Fixes #53056 Fixes CVE-2022-30580 Change-Id: I03205172aef3801c3194f5098bdb93290c02b1b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/403759 Reviewed-by: Bryan Mills Reviewed-by: Roland Shoemaker (cherry picked from commit 960ffa98ce73ef2c2060c84c7ac28d37a83f345e) Reviewed-on: https://go-review.googlesource.com/c/go/+/408578 Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot --- src/os/exec/exec.go | 3 +++ src/os/exec/exec_test.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index 0c49575511..505de58e84 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -374,6 +374,9 @@ func lookExtensions(path, dir string) (string, error) { // The Wait method will return the exit code and release associated resources // once the command exits. func (c *Cmd) Start() error { + if c.Path == "" && c.lookPathErr == nil { + c.lookPathErr = errors.New("exec: no command") + } if c.lookPathErr != nil { c.closeDescriptors(c.closeAfterStart) c.closeDescriptors(c.closeAfterWait) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index d854e0de84..a951be718d 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -1156,3 +1156,11 @@ func TestChildCriticalEnv(t *testing.T) { t.Error("no SYSTEMROOT found") } } + +func TestNoPath(t *testing.T) { + err := new(exec.Cmd).Start() + want := "exec: no command" + if err == nil || err.Error() != want { + t.Errorf("new(Cmd).Start() = %v, want %q", err, want) + } +} -- cgit v1.2.3-54-g00ecf From c15a8e2dbb5ac376a6ed890735341b812d6b965c Mon Sep 17 00:00:00 2001 From: Tatiana Bradley Date: Thu, 12 May 2022 14:58:29 -0400 Subject: [release-branch.go1.17] crypto/tls: randomly generate ticket_age_add As required by RFC 8446, section 4.6.1, ticket_age_add now holds a random 32-bit value. Before this change, this value was always set to 0. This change also documents the reasoning for always setting ticket_nonce to 0. The value ticket_nonce must be unique per connection, but we only ever send one ticket per connection. Updates #52814 Fixes #52832 Fixes CVE-2022-30629 Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb Reviewed-on: https://go-review.googlesource.com/c/go/+/405994 Reviewed-by: Tatiana Bradley Reviewed-by: Roland Shoemaker Run-TryBot: Tatiana Bradley TryBot-Result: Gopher Robot (cherry picked from commit fe4de36198794c447fbd9d7cc2d7199a506c76a5) Reviewed-on: https://go-review.googlesource.com/c/go/+/408574 Run-TryBot: Roland Shoemaker --- src/crypto/tls/handshake_server_tls13.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go index 08251b84de..6aa52698a3 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -10,6 +10,7 @@ import ( "crypto" "crypto/hmac" "crypto/rsa" + "encoding/binary" "errors" "hash" "io" @@ -741,6 +742,19 @@ func (hs *serverHandshakeStateTLS13) sendSessionTickets() error { } m.lifetime = uint32(maxSessionTicketLifetime / time.Second) + // ticket_age_add is a random 32-bit value. See RFC 8446, section 4.6.1 + // The value is not stored anywhere; we never need to check the ticket age + // because 0-RTT is not supported. + ageAdd := make([]byte, 4) + _, err = hs.c.config.rand().Read(ageAdd) + if err != nil { + return err + } + m.ageAdd = binary.LittleEndian.Uint32(ageAdd) + + // ticket_nonce, which must be unique per connection, is always left at + // zero because we only ever send one ticket per connection. + if _, err := c.writeRecord(recordTypeHandshake, m.marshal()); err != nil { return err } -- cgit v1.2.3-54-g00ecf From 03c2e56f686e9bf648d606bc06fe6bbb263a3661 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 17 Jan 2022 21:54:17 +0000 Subject: [release-branch.go1.17] crypto/tls: avoid extra allocations in steady-state Handshake calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Read and Write methods of *tls.Conn call Handshake unconditionally, every time they are called, expecting it to only perform a new handshake if required. However in go 1.17 handshakeContext was extended to set up a cancelable Context, and importantly did so prior to checking if a handshake is required. This thus causes it to allocate on every call, even in those that are no-ops when called in a Read or Write on an established connection, sometimes leading to very large numbers of allocations during reads. This change adds an early return, prior to setting up the context or proceeding into the handshakeMutex and checking the handshake error, if the handshake status atomic indicates handshake is already complete. name old allocs/op new allocs/op delta Throughput/MaxPacket/1MB/TLSv12-10 1.07k ± 0% 0.62k ± 0% -42.16% (p=0.000 n=4+5) Throughput/MaxPacket/1MB/TLSv13-10 1.70k ± 0% 1.25k ± 0% ~ (p=0.079 n=4+5) Throughput/MaxPacket/2MB/TLSv12-10 1.62k ± 0% 0.73k ± 0% -55.18% (p=0.008 n=5+5) Throughput/MaxPacket/2MB/TLSv13-10 2.26k ± 0% 1.36k ± 0% -39.64% (p=0.008 n=5+5) Throughput/MaxPacket/4MB/TLSv12-10 2.74k ± 0% 0.95k ± 0% -65.35% (p=0.008 n=5+5) Throughput/MaxPacket/4MB/TLSv13-10 3.37k ± 0% 1.58k ± 0% -53.15% (p=0.008 n=5+5) Throughput/MaxPacket/8MB/TLSv12-10 4.96k ± 0% 1.39k ± 0% -72.06% (p=0.016 n=4+5) Throughput/MaxPacket/8MB/TLSv13-10 5.60k ± 0% 2.01k ± 0% -64.05% (p=0.008 n=5+5) Throughput/MaxPacket/16MB/TLSv12-10 9.42k ± 0% 2.27k ± 1% -75.92% (p=0.016 n=4+5) Throughput/MaxPacket/16MB/TLSv13-10 10.0k ± 0% 2.9k ± 0% -71.39% (p=0.008 n=5+5) Throughput/MaxPacket/32MB/TLSv12-10 18.3k ± 0% 4.0k ± 0% -77.97% (p=0.008 n=5+5) Throughput/MaxPacket/32MB/TLSv13-10 18.9k ± 0% 4.6k ± 0% -75.62% (p=0.008 n=5+5) Throughput/MaxPacket/64MB/TLSv12-10 36.2k ± 0% 7.5k ± 0% -79.15% (p=0.008 n=5+5) Throughput/MaxPacket/64MB/TLSv13-10 36.7k ± 0% 8.1k ± 0% -78.06% (p=0.008 n=5+5) Throughput/DynamicPacket/1MB/TLSv12-10 1.12k ± 0% 0.63k ± 0% -44.20% (p=0.008 n=5+5) Throughput/DynamicPacket/1MB/TLSv13-10 1.76k ± 0% 1.26k ± 0% -28.22% (p=0.016 n=5+4) Throughput/DynamicPacket/2MB/TLSv12-10 1.68k ± 0% 0.74k ± 0% -56.11% (p=0.008 n=5+5) Throughput/DynamicPacket/2MB/TLSv13-10 2.32k ± 0% 1.37k ± 0% -40.80% (p=0.008 n=5+5) Throughput/DynamicPacket/4MB/TLSv12-10 2.80k ± 0% 0.96k ± 0% -65.81% (p=0.008 n=5+5) Throughput/DynamicPacket/4MB/TLSv13-10 3.43k ± 0% 1.59k ± 0% -53.57% (p=0.008 n=5+5) Throughput/DynamicPacket/8MB/TLSv12-10 5.03k ± 0% 1.39k ± 0% -72.27% (p=0.008 n=5+5) Throughput/DynamicPacket/8MB/TLSv13-10 5.66k ± 0% 2.02k ± 0% -64.27% (p=0.008 n=5+5) Throughput/DynamicPacket/16MB/TLSv12-10 9.48k ± 0% 2.28k ± 1% -75.98% (p=0.008 n=5+5) Throughput/DynamicPacket/16MB/TLSv13-10 10.1k ± 0% 2.9k ± 0% -71.34% (p=0.008 n=5+5) Throughput/DynamicPacket/32MB/TLSv12-10 18.4k ± 0% 4.0k ± 0% -78.13% (p=0.008 n=5+5) Throughput/DynamicPacket/32MB/TLSv13-10 19.0k ± 0% 4.6k ± 0% -75.54% (p=0.008 n=5+5) Throughput/DynamicPacket/64MB/TLSv12-10 36.2k ± 0% 7.6k ± 1% -79.02% (p=0.008 n=5+5) Throughput/DynamicPacket/64MB/TLSv13-10 36.8k ± 0% 8.2k ± 1% -77.76% (p=0.008 n=5+5) Fixes #52790 Change-Id: Iacb1f9bf7802022960d9dbce141b8c0587a614d4 Reviewed-on: https://go-review.googlesource.com/c/go/+/379034 Reviewed-by: David Chase TryBot-Result: Gopher Robot Reviewed-by: Filippo Valsorda Auto-Submit: Filippo Valsorda Run-TryBot: Filippo Valsorda (cherry picked from commit a4af35607536b2b0d73be94df188b9f5a157480c) Reviewed-on: https://go-review.googlesource.com/c/go/+/405544 Reviewed-by: Roland Shoemaker Reviewed-by: Alex Rakoczy --- src/crypto/tls/conn.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go index 969f357834..d0a2550cbc 100644 --- a/src/crypto/tls/conn.go +++ b/src/crypto/tls/conn.go @@ -32,6 +32,7 @@ type Conn struct { // handshakeStatus is 1 if the connection is currently transferring // application data (i.e. is not currently processing a handshake). + // handshakeStatus == 1 implies handshakeErr == nil. // This field is only to be accessed with sync/atomic. handshakeStatus uint32 // constant after handshake; protected by handshakeMutex @@ -1396,6 +1397,13 @@ func (c *Conn) HandshakeContext(ctx context.Context) error { } func (c *Conn) handshakeContext(ctx context.Context) (ret error) { + // Fast sync/atomic-based exit if there is no handshake in flight and the + // last one succeeded without an error. Avoids the expensive context setup + // and mutex for most Read and Write calls. + if c.handshakeComplete() { + return nil + } + handshakeCtx, cancel := context.WithCancel(ctx) // Note: defer this before starting the "interrupter" goroutine // so that we can tell the difference between the input being canceled and @@ -1454,6 +1462,9 @@ func (c *Conn) handshakeContext(ctx context.Context) (ret error) { if c.handshakeErr == nil && !c.handshakeComplete() { c.handshakeErr = errors.New("tls: internal error: handshake should have had a result") } + if c.handshakeErr != nil && c.handshakeComplete() { + panic("tls: internal error: handshake returned an error but is marked successful") + } return c.handshakeErr } -- cgit v1.2.3-54-g00ecf From 909881db03b7aca794c791e4c6e893c9a4638521 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 27 May 2022 11:42:59 -0400 Subject: [release-branch.go1.17] misc/cgo/testsanitizers: buffer the signal channel in TestTSAN/tsan11 This fix is analogous to the one in CL 407888. 'go vet' catches the error, but it is not run on this file because the file is (only) compiled when running testsanitizers/TestTSAN. Fixes #53114. Updates #53113. Change-Id: I74f7b7390a9775ff00a06214c1019ba28846dd11 Reviewed-on: https://go-review.googlesource.com/c/go/+/409094 Auto-Submit: Bryan Mills Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills (cherry picked from commit 0f57c88bce9899a91686646a1e9cd7aae55911ef) Reviewed-on: https://go-review.googlesource.com/c/go/+/408824 Reviewed-by: Ian Lance Taylor --- misc/cgo/testsanitizers/testdata/tsan11.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/cgo/testsanitizers/testdata/tsan11.go b/misc/cgo/testsanitizers/testdata/tsan11.go index 70ac9c8ae2..189e10f699 100644 --- a/misc/cgo/testsanitizers/testdata/tsan11.go +++ b/misc/cgo/testsanitizers/testdata/tsan11.go @@ -45,7 +45,7 @@ static void register_handler(int signo) { import "C" func main() { - ch := make(chan os.Signal) + ch := make(chan os.Signal, 1) signal.Notify(ch, syscall.SIGUSR2) C.register_handler(C.int(syscall.SIGUSR1)) -- cgit v1.2.3-54-g00ecf From 4c69fd51a9ed70da0a6399d0b084b828bc30d562 Mon Sep 17 00:00:00 2001 From: Yasuhiro Matsumoto Date: Fri, 22 Apr 2022 10:07:51 +0900 Subject: [release-branch.go1.17] path/filepath: do not remove prefix "." when following path contains ":". For #52476 Fixes #52478 Fixes CVE-2022-29804 Change-Id: I9eb72ac7dbccd6322d060291f31831dc389eb9bb Reviewed-on: https://go-review.googlesource.com/c/go/+/401595 Auto-Submit: Ian Lance Taylor Reviewed-by: Alex Brainman Run-TryBot: Ian Lance Taylor Reviewed-by: Ian Lance Taylor Reviewed-by: Damien Neil TryBot-Result: Gopher Robot Reviewed-on: https://go-review.googlesource.com/c/go/+/405235 Reviewed-by: Yasuhiro Matsumoto --- src/path/filepath/path.go | 14 +++++++++++++- src/path/filepath/path_test.go | 3 +++ src/path/filepath/path_windows_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go index b56534dead..8300a32cb1 100644 --- a/src/path/filepath/path.go +++ b/src/path/filepath/path.go @@ -117,9 +117,21 @@ func Clean(path string) string { case os.IsPathSeparator(path[r]): // empty path element r++ - case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])): + case path[r] == '.' && r+1 == n: // . element r++ + case path[r] == '.' && os.IsPathSeparator(path[r+1]): + // ./ element + r++ + + for r < len(path) && os.IsPathSeparator(path[r]) { + r++ + } + if out.w == 0 && volumeNameLen(path[r:]) > 0 { + // When joining prefix "." and an absolute path on Windows, + // the prefix should not be removed. + out.append('.') + } case path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])): // .. element: remove to last separator r += 2 diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index bc5509b49c..ed17a8854d 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -93,6 +93,9 @@ var wincleantests = []PathTest{ {`//host/share/foo/../baz`, `\\host\share\baz`}, {`\\a\b\..\c`, `\\a\b\c`}, {`\\a\b`, `\\a\b`}, + {`.\c:`, `.\c:`}, + {`.\c:\foo`, `.\c:\foo`}, + {`.\c:foo`, `.\c:foo`}, } func TestClean(t *testing.T) { diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go index 76a459ac96..3edafb5a85 100644 --- a/src/path/filepath/path_windows_test.go +++ b/src/path/filepath/path_windows_test.go @@ -530,3 +530,29 @@ func TestNTNamespaceSymlink(t *testing.T) { t.Errorf(`EvalSymlinks(%q): got %q, want %q`, filelink, got, want) } } + +func TestIssue52476(t *testing.T) { + tests := []struct { + lhs, rhs string + want string + }{ + {`..\.`, `C:`, `..\C:`}, + {`..`, `C:`, `..\C:`}, + {`.`, `:`, `:`}, + {`.`, `C:`, `.\C:`}, + {`.`, `C:/a/b/../c`, `.\C:\a\c`}, + {`.`, `\C:`, `.\C:`}, + {`C:\`, `.`, `C:\`}, + {`C:\`, `C:\`, `C:\C:`}, + {`C`, `:`, `C\:`}, + {`\.`, `C:`, `\C:`}, + {`\`, `C:`, `\C:`}, + } + + for _, test := range tests { + got := filepath.Join(test.lhs, test.rhs) + if got != test.want { + t.Errorf(`Join(%q, %q): got %q, want %q`, test.lhs, test.rhs, got, test.want) + } + } +} -- cgit v1.2.3-54-g00ecf From 26cdea3acca29db94541236f0037a20aa22ce2d7 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Wed, 1 Jun 2022 09:39:36 -0400 Subject: [release-branch.go1.17] go1.17.11 Change-Id: If6011b195277160ea0f5c2d13bb2d9ea9265145f Reviewed-on: https://go-review.googlesource.com/c/go/+/409736 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov Reviewed-by: Alex Rakoczy --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index d1ab52a8a2..3e9980643e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.17.10 \ No newline at end of file +go1.17.11 \ No newline at end of file -- cgit v1.2.3-54-g00ecf