From db5cb5fceed48a487ca15c8321ddcb19652cc1b9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 25 Oct 2022 23:10:13 -0400 Subject: [release-branch.go1.18] crypto/x509: respect GODEBUG changes for allowing SHA1 certificates This allows programs that want SHA1 support to call os.Setenv at startup instead of insisting that users set the environment variable themselves. For #41682. Fixes #56436. Fixes #56437. Change-Id: Idcb96212a1d8c560e1dd8eaf7c80b6266f16431e Reviewed-on: https://go-review.googlesource.com/c/go/+/445496 Reviewed-by: David Chase Run-TryBot: Russ Cox Reviewed-by: Roland Shoemaker TryBot-Result: Gopher Robot Auto-Submit: Russ Cox Reviewed-on: https://go-review.googlesource.com/c/go/+/445656 --- src/crypto/x509/verify_test.go | 4 ++-- src/crypto/x509/x509.go | 8 +++----- src/crypto/x509/x509_test.go | 7 ++----- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go index 100a8ff0f9..c3ef503b27 100644 --- a/src/crypto/x509/verify_test.go +++ b/src/crypto/x509/verify_test.go @@ -540,8 +540,8 @@ func testVerify(t *testing.T, test verifyTest, useSystemRoots bool) { func TestGoVerify(t *testing.T) { // Temporarily enable SHA-1 verification since a number of test chains // require it. TODO(filippo): regenerate test chains. - defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1) - debugAllowSHA1 = true + t.Setenv("GODEBUG", "x509sha1=1") + for _, test := range verifyTests { t.Run(test.name, func(t *testing.T) { testVerify(t, test, false) diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go index 85720b3ccb..dc470f53f2 100644 --- a/src/crypto/x509/x509.go +++ b/src/crypto/x509/x509.go @@ -730,9 +730,6 @@ type Certificate struct { // involves algorithms that are not currently implemented. var ErrUnsupportedAlgorithm = errors.New("x509: cannot verify signature: algorithm unimplemented") -// debugAllowSHA1 allows SHA-1 signatures. See issue 41682. -var debugAllowSHA1 = godebug.Get("x509sha1") == "1" - // An InsecureAlgorithmError indicates that the SignatureAlgorithm used to // generate the signature is not secure, and the signature has been rejected. // @@ -792,7 +789,7 @@ func (c *Certificate) CheckSignatureFrom(parent *Certificate) error { // TODO(agl): don't ignore the path length constraint. - return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificate, c.Signature, parent.PublicKey, debugAllowSHA1) + return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificate, c.Signature, parent.PublicKey, false) } // CheckSignature verifies that signature is a valid signature over signed from @@ -839,7 +836,8 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey case crypto.MD5: return InsecureAlgorithmError(algo) case crypto.SHA1: - if !allowSHA1 { + // SHA-1 signatures are mostly disabled. See go.dev/issue/41682. + if !allowSHA1 && godebug.Get("x509sha1") != "1" { return InsecureAlgorithmError(algo) } fallthrough diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go index 6c1e95a203..48a864057d 100644 --- a/src/crypto/x509/x509_test.go +++ b/src/crypto/x509/x509_test.go @@ -1864,9 +1864,7 @@ func TestSHA1(t *testing.T) { t.Fatalf("certificate verification returned %v (%T), wanted InsecureAlgorithmError", err, err) } - defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1) - debugAllowSHA1 = true - + t.Setenv("GODEBUG", "x509sha1=1") if err = cert.CheckSignatureFrom(cert); err != nil { t.Fatalf("SHA-1 certificate did not verify with GODEBUG=x509sha1=1: %v", err) } @@ -3335,8 +3333,7 @@ func TestLargeOID(t *testing.T) { } func TestDisableSHA1ForCertOnly(t *testing.T) { - defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1) - debugAllowSHA1 = false + t.Setenv("GODEBUG", "") tmpl := &Certificate{ SerialNumber: big.NewInt(1), -- cgit v1.2.1 From e54e808d781745e12b0f7ce471934735750e7015 Mon Sep 17 00:00:00 2001 From: "Paul E. Murphy" Date: Thu, 20 Oct 2022 17:01:01 -0500 Subject: [release-branch.go1.18] runtime: fix usleep on linux/PPC64 The existing implementation fails to convert the remainder microseconds to nanoseconds. This causes sysmon to consume much more cpu, and generate lots of context switches. We can also do a little better here to avoid division by a constant. I used go to determine the magic numbers. Fixes #56396 Change-Id: I2e37ec218b9027efab6db4634eed1504c0c1b3c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/444735 Reviewed-by: Lynn Boger Run-TryBot: Paul Murphy Reviewed-by: David Chase TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Reviewed-on: https://go-review.googlesource.com/c/go/+/445156 --- src/runtime/sys_linux_ppc64x.s | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/runtime/sys_linux_ppc64x.s b/src/runtime/sys_linux_ppc64x.s index 01d6c85597..b86a820ab2 100644 --- a/src/runtime/sys_linux_ppc64x.s +++ b/src/runtime/sys_linux_ppc64x.s @@ -119,16 +119,23 @@ TEXT runtime·pipe2(SB),NOSPLIT|NOFRAME,$0-20 MOVW R3, errno+16(FP) RET +// func usleep(usec uint32) TEXT runtime·usleep(SB),NOSPLIT,$16-4 MOVW usec+0(FP), R3 - MOVD R3, R5 - MOVW $1000000, R4 - DIVD R4, R3 - MOVD R3, 8(R1) - MOVW $1000, R4 - MULLD R3, R4 - SUB R4, R5 - MOVD R5, 16(R1) + + // Use magic constant 0x8637bd06 and shift right 51 + // to perform usec/1000000. + ORIS $0x8637, R0, R4 // Note, R0 always contains 0 here. + OR $0xbd06, R4, R4 + MULLD R3, R4, R4 // Convert usec to S. + SRD $51, R4, R4 + MOVD R4, 8(R1) // Store to tv_sec + + MOVD $1000000, R5 + MULLW R4, R5, R5 // Convert tv_sec back into uS + SUB R5, R3, R5 // Compute remainder uS. + MULLD $1000, R5, R5 // Convert to nsec + MOVD R5, 16(R1) // Store to tv_nsec // nanosleep(&ts, 0) ADD $8, R1, R3 -- cgit v1.2.1 From d9c62ce86a3fd42704f3804d3e31157c57b1f7b3 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 14 Oct 2022 19:52:00 -0400 Subject: [release-branch.go1.18] cmd/compile: copy blank parameter node when substituting function type When a function type is copied (e.g. for substituting type parameters), we make copies of its parameter ir.Name nodes, so they are not shared with the old function type. But currently a blank (_) identifier is not copied but shared. The parameter node's frame offset is assigned (in ABI analysis) and then used in the concurrent backend. Shared node can cause a data race. Make a new blank parameter node to avoid sharing. (Unified IR does already not have this problem. This fixes non-unified-IR mode.) Updates #55357. Fixes #56359. Change-Id: Ie27f08e5589ac7d5d3f0d0d5de1a21e4fd2765c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/443158 Reviewed-by: Keith Randall Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot Reviewed-by: Keith Randall (cherry picked from commit 4725c71b735143a138b24f2b0e055c717d8d69ca) Reviewed-on: https://go-review.googlesource.com/c/go/+/445177 --- src/cmd/compile/internal/test/race.go | 65 ++++++++++++++++++++++++++++++ src/cmd/compile/internal/typecheck/subr.go | 10 ++++- 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/cmd/compile/internal/test/race.go diff --git a/src/cmd/compile/internal/test/race.go b/src/cmd/compile/internal/test/race.go new file mode 100644 index 0000000000..4cc4d53609 --- /dev/null +++ b/src/cmd/compile/internal/test/race.go @@ -0,0 +1,65 @@ +// Copyright 2022 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. + +//go:build !compiler_bootstrap +// +build !compiler_bootstrap + +package test + +// The racecompile builder only builds packages, but does not build +// or run tests. This is a non-test file to hold cases that (used +// to) trigger compiler data races, so they will be exercised on +// the racecompile builder. +// +// This package is not imported so functions here are not included +// in the actual compiler. + +// Issue 55357: data race when building multiple instantiations of +// generic closures with _ parameters. +func Issue55357() { + type U struct { + A int + B string + C string + } + var q T55357[U] + q.Count() + q.List() + + type M struct { + A int64 + B uint32 + C uint32 + } + var q2 T55357[M] + q2.Count() + q2.List() +} + +type T55357[T any] struct{} + +//go:noinline +func (q *T55357[T]) do(w, v bool, fn func(bk []byte, v T) error) error { + return nil +} + +func (q *T55357[T]) Count() (n int, rerr error) { + err := q.do(false, false, func(kb []byte, _ T) error { + n++ + return nil + }) + return n, err +} + +func (q *T55357[T]) List() (list []T, rerr error) { + var l []T + err := q.do(false, true, func(_ []byte, v T) error { + l = append(l, v) + return nil + }) + if err != nil { + return nil, err + } + return l, nil +} diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index a41a3d62fb..7c48fb577d 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -1356,7 +1356,8 @@ func (ts *Tsubster) tstruct(t *types.Type, force bool) *types.Type { newfields[i].SetNointerface(true) } if f.Nname != nil && ts.Vars != nil { - v := ts.Vars[f.Nname.(*ir.Name)] + n := f.Nname.(*ir.Name) + v := ts.Vars[n] if v != nil { // This is the case where we are // translating the type of the function we @@ -1364,6 +1365,13 @@ func (ts *Tsubster) tstruct(t *types.Type, force bool) *types.Type { // the subst.ts.vars table, and we want to // change to reference the new dcl. newfields[i].Nname = v + } else if ir.IsBlank(n) { + // Blank variable is not dcl list. Make a + // new one to not share. + m := ir.NewNameAt(n.Pos(), ir.BlankNode.Sym()) + m.SetType(n.Type()) + m.SetTypecheck(1) + newfields[i].Nname = m } else { // This is the case where we are // translating the type of a function -- cgit v1.2.1 From ca571803a87968b280d74b03cbc820ecf68e069d Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 3 Nov 2022 11:02:51 -0700 Subject: [release-branch.go1.18] os/exec: allow NUL in environment variables on Plan 9 Plan 9 uses NUL as os.PathListSeparator, so it's almost always going to appear in the environment variable list. Exempt GOOS=plan9 from the check for NUL in environment variables. For #56284. For #56544. Fixes #56550. Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a Reviewed-on: https://go-review.googlesource.com/c/go/+/447715 Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Damien Neil Reviewed-on: https://go-review.googlesource.com/c/go/+/447875 Run-TryBot: David du Colombier <0intro@gmail.com> --- src/os/exec/env_test.go | 9 ++++++++- src/os/exec/exec.go | 14 +++++++++----- src/os/exec/exec_test.go | 3 +++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/os/exec/env_test.go b/src/os/exec/env_test.go index 47b7c04705..43d14fb56d 100644 --- a/src/os/exec/env_test.go +++ b/src/os/exec/env_test.go @@ -12,6 +12,7 @@ import ( func TestDedupEnv(t *testing.T) { tests := []struct { noCase bool + nulOK bool in []string want []string wantErr bool @@ -36,9 +37,15 @@ func TestDedupEnv(t *testing.T) { want: []string{"B=b"}, wantErr: true, }, + { + // Plan 9 needs to preserve environment variables with NUL (#56544). + nulOK: true, + in: []string{"path=one\x00two"}, + want: []string{"path=one\x00two"}, + }, } for _, tt := range tests { - got, err := dedupEnvCase(tt.noCase, tt.in) + got, err := dedupEnvCase(tt.noCase, tt.nulOK, tt.in) if !reflect.DeepEqual(got, tt.want) || (err != nil) != tt.wantErr { t.Errorf("Dedup(%v, %q) = %q, %v; want %q, error:%v", tt.noCase, tt.in, got, err, tt.want, tt.wantErr) } diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index 056a7e07d7..e1147b75fa 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -745,23 +745,27 @@ func minInt(a, b int) int { // dedupEnv returns a copy of env with any duplicates removed, in favor of // later values. // Items not of the normal environment "key=value" form are preserved unchanged. -// Items containing NUL characters are removed, and an error is returned along with -// the remaining values. +// Except on Plan 9, items containing NUL characters are removed, and +// an error is returned along with the remaining values. func dedupEnv(env []string) ([]string, error) { - return dedupEnvCase(runtime.GOOS == "windows", env) + return dedupEnvCase(runtime.GOOS == "windows", runtime.GOOS == "plan9", env) } // dedupEnvCase is dedupEnv with a case option for testing. // If caseInsensitive is true, the case of keys is ignored. -func dedupEnvCase(caseInsensitive bool, env []string) ([]string, error) { +// If nulOK is false, items containing NUL characters are allowed. +func dedupEnvCase(caseInsensitive, nulOK bool, env []string) ([]string, error) { var err error out := make([]string, 0, len(env)) saw := make(map[string]int, len(env)) // key => index into out for _, kv := range env { - if strings.IndexByte(kv, 0) != -1 { + // Reject NUL in environment variables to prevent security issues (#56284); + // except on Plan 9, which uses NUL as os.PathListSeparator (#56544). + if !nulOK && strings.IndexByte(kv, 0) != -1 { err = errors.New("exec: environment variable contains NUL") continue } + k, _, ok := strings.Cut(kv, "=") if !ok { out = append(out, kv) diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 0be8c6cc18..99bc0329d1 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -1030,6 +1030,9 @@ func TestDedupEnvEcho(t *testing.T) { } func TestEnvNULCharacter(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip("plan9 explicitly allows NUL in the enviroment") + } cmd := helperCommand(t, "echoenv", "FOO", "BAR") cmd.Env = append(cmd.Env, "FOO=foo\x00BAR=bar") out, err := cmd.CombinedOutput() -- cgit v1.2.1 From d2a7a180d38d4a51eb5d8f86bb71bed0ecee09ce Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Thu, 3 Nov 2022 11:56:43 -0700 Subject: [release-branch.go1.18] cmd/compile: allow ineffectual //go:linkname in -lang=go1.17 and older Prior to Go 1.18, ineffectual //go:linkname directives (i.e., directives referring to an undeclared name, or to a declared type or constant) were treated as noops. In Go 1.18, we changed this into a compiler error to mitigate accidental misuse. However, the x/sys repo contained ineffectual //go:linkname directives up until go.dev/cl/274573, which has caused a lot of user confusion. It seems a bit late to worry about now, but to at least prevent further user pain, this CL changes the error message to only apply to modules using "go 1.18" or newer. (The x/sys repo declared "go 1.12" at the time go.dev/cl/274573 was submitted.) For #55889. Fixes #56556. Change-Id: Id762fff96fd13ba0f1e696929a9e276dfcba2620 Reviewed-on: https://go-review.googlesource.com/c/go/+/447755 TryBot-Result: Gopher Robot Run-TryBot: Matthew Dempsky Reviewed-by: Bryan Mills Reviewed-on: https://go-review.googlesource.com/c/go/+/447815 --- src/cmd/compile/internal/noder/noder.go | 4 +++- test/fixedbugs/issue55889.go | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue55889.go diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index b36db67a50..17ec87788a 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/src/cmd/compile/internal/noder/noder.go @@ -323,7 +323,9 @@ func (p *noder) processPragmas() { } n := ir.AsNode(typecheck.Lookup(l.local).Def) if n == nil || n.Op() != ir.ONAME { - p.errorAt(l.pos, "//go:linkname must refer to declared function or variable") + if types.AllowsGoVersion(types.LocalPkg, 1, 18) { + p.errorAt(l.pos, "//go:linkname must refer to declared function or variable") + } continue } if n.Sym().Linkname != "" { diff --git a/test/fixedbugs/issue55889.go b/test/fixedbugs/issue55889.go new file mode 100644 index 0000000000..68812c2157 --- /dev/null +++ b/test/fixedbugs/issue55889.go @@ -0,0 +1,21 @@ +// errorcheck -0 -lang=go1.17 + +// Copyright 2022 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. + +// Prior to Go 1.18, ineffectual //go:linkname directives were treated +// as noops. Ensure that modules that contain these directives (e.g., +// x/sys prior to go.dev/cl/274573) continue to compile. + +package p + +import _ "unsafe" + +//go:linkname nonexistent nonexistent + +//go:linkname constant constant +const constant = 42 + +//go:linkname typename typename +type typename int -- cgit v1.2.1 From a3dce1276887df0de970cac8524b822c092ab39f Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Fri, 30 Sep 2022 19:01:09 -0400 Subject: [release-branch.go1.18] runtime: don't jump stack if at entry of systemstack The traceback code has special "jump stack" logic, to trace back stack switches through systemstack. If we're at the entry of systemstack, the stack switch hasn't happened, so don't jump to user stack. The jump stack logic is only used if we're on the g0 stack. It can happen that we're at the entry of a recursive systemstack call on the g0 stack. In we jump stack here, there will be two problems: 1. There are frames between entering the g0 stack and this recursive systemstack call. Those frames will be lost. 2. Worse, we switched frame.sp but frame.fp calculation will use the entry SP delta (0), which will be wrong, which in turn leads wrong frame.lr and things will go off. For now, don't jump stack if we're at entry of systemstack (SP delta is 0). Using a per-PC SPWRITE marker may be a better fix. If we haven't written the SP, we haven't switched the stack so we can just unwind like a normal function. Updates #55851. Fixes #56635. Change-Id: I2b624c8c086b235b34d9c7d3cebd4a37264f00f8 Reviewed-on: https://go-review.googlesource.com/c/go/+/437299 TryBot-Result: Gopher Robot Run-TryBot: Cherry Mui Reviewed-by: Michael Pratt (cherry picked from commit 500bc6b8056ee2eaf7640610a48ffa00bdd896a5) Reviewed-on: https://go-review.googlesource.com/c/go/+/448517 --- src/runtime/traceback.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 56128dc882..b8f580f084 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -182,6 +182,17 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in case funcID_systemstack: // systemstack returns normally, so just follow the // stack transition. + if usesLR && funcspdelta(f, frame.pc, &cache) == 0 { + // We're at the function prologue and the stack + // switch hasn't happened, or epilogue where we're + // about to return. Just unwind normally. + // Do this only on LR machines because on x86 + // systemstack doesn't have an SP delta (the CALL + // instruction opens the frame), therefore no way + // to check. + flag &^= funcFlag_SPWRITE + break + } frame.sp = gp.m.curg.sched.sp stack = gp.m.curg.stack cgoCtxt = gp.m.curg.cgoCtxt -- cgit v1.2.1 From d96e584773bf0577a4da2ffc9543111ddd80627c Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 11 Nov 2022 17:26:45 +0000 Subject: [release-branch.go1.18] net: disable TestLookupDotsWithRemoteSource and TestLookupGoogleSRV These tests fail consistently due to a DNS change causing widespread trybot outages. For #56707. Fixes #56709. Change-Id: Iebdf91254a922a48880021198f0f12f6bc16b6e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/449640 Reviewed-by: Carlos Amedee Reviewed-by: Bryan Mills Run-TryBot: Michael Knyszek TryBot-Result: Gopher Robot Reviewed-by: Damien Neil (cherry picked from commit 97765249082b6835c77517a4e63bb38cfd6db97b) Reviewed-on: https://go-review.googlesource.com/c/go/+/449505 Reviewed-by: Ian Lance Taylor --- src/net/lookup_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index 3a31f56bea..655543c7d2 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -71,6 +71,10 @@ var lookupGoogleSRVTests = []struct { var backoffDuration = [...]time.Duration{time.Second, 5 * time.Second, 30 * time.Second} func TestLookupGoogleSRV(t *testing.T) { + // TODO(mknyszek): Figure out next steps for this test. This is just + // a quick fix. + t.Skip("fails consistently due to an upstream DNS change; see #56707.") + t.Parallel() mustHaveExternalNetwork(t) @@ -631,6 +635,10 @@ func TestLookupDotsWithLocalSource(t *testing.T) { } func TestLookupDotsWithRemoteSource(t *testing.T) { + // TODO(mknyszek): Figure out next steps for this test. This is just + // a quick fix. + t.Skip("fails consistently due to an upstream DNS change; see #56707.") + if runtime.GOOS == "darwin" || runtime.GOOS == "ios" { testenv.SkipFlaky(t, 27992) } -- cgit v1.2.1 From 3115ed23bdaa3dcc465feba4100da40c2d22f964 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Wed, 9 Nov 2022 10:55:54 -0500 Subject: [release-branch.go1.18] runtime: make GC see object as allocated after it is initialized When the GC is scanning some memory (possibly conservatively), finding a pointer, while concurrently another goroutine is allocating an object at the same address as the found pointer, the GC may see the pointer before the object and/or the heap bits are initialized. This may cause the GC to see bad pointers and possibly crash. To prevent this, we make it that the scanner can only see the object as allocated after the object and the heap bits are initialized. Currently the allocator uses freeindex to find the next available slot, and that code is coupled with updating the free index to a new slot past it. The scanner also uses the freeindex to determine if an object is allocated. This is somewhat racy. This CL makes the scanner use a different field, which is only updated after the object initialization (and a memory barrier). Updates #54596. Fixes #56751. Change-Id: I2a57a226369926e7192c253dd0d21d3faf22297c Reviewed-on: https://go-review.googlesource.com/c/go/+/449017 Reviewed-by: Austin Clements Reviewed-by: Michael Knyszek Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot (cherry picked from commit febe7b8e2a4dd7cce6ab8d02cf79a5430819cbe5) Reviewed-on: https://go-review.googlesource.com/c/go/+/453255 --- src/runtime/malloc.go | 10 ++++++++++ src/runtime/mbitmap.go | 2 +- src/runtime/mgcsweep.go | 1 + src/runtime/mheap.go | 10 ++++++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 6ed6ceade2..d738644c7e 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -1135,6 +1135,16 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { // the garbage collector could follow a pointer to x, // but see uninitialized memory or stale heap bits. publicationBarrier() + // As x and the heap bits are initialized, update + // freeIndexForScan now so x is seen by the GC + // (including convervative scan) as an allocated object. + // While this pointer can't escape into user code as a + // _live_ pointer until we return, conservative scanning + // may find a dead pointer that happens to point into this + // object. Delaying this update until now ensures that + // conservative scanning considers this pointer dead until + // this point. + span.freeIndexForScan = span.freeindex // Allocate black during GC. // All slots hold nil so no scanning is needed. diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go index 937968807b..95d88d8c61 100644 --- a/src/runtime/mbitmap.go +++ b/src/runtime/mbitmap.go @@ -220,7 +220,7 @@ func (s *mspan) nextFreeIndex() uintptr { // been no preemption points since ensuring this (which could allow a // GC transition, which would allow the state to change). func (s *mspan) isFree(index uintptr) bool { - if index < s.freeindex { + if index < s.freeIndexForScan { return false } bytep, mask := s.allocBits.bitp(index) diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 0d58f8e0b5..2aa670e1b8 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -623,6 +623,7 @@ func (sl *sweepLocked) sweep(preserve bool) bool { s.allocCount = nalloc s.freeindex = 0 // reset allocation index to start of span. + s.freeIndexForScan = 0 if trace.enabled { getg().m.p.ptr().traceReclaimed += uintptr(nfreed) * s.elemsize } diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index ecbd0a3a49..134387562e 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -459,6 +459,14 @@ type mspan struct { limit uintptr // end of data in span speciallock mutex // guards specials list specials *special // linked list of special records sorted by offset. + + // freeIndexForScan is like freeindex, except that freeindex is + // used by the allocator whereas freeIndexForScan is used by the + // GC scanner. They are two fields so that the GC sees the object + // is allocated only when the object and the heap bits are + // initialized (see also the assignment of freeIndexForScan in + // mallocgc, and issue 54596). + freeIndexForScan uintptr } func (s *mspan) base() uintptr { @@ -1250,6 +1258,7 @@ HaveSpan: // Initialize mark and allocation structures. s.freeindex = 0 + s.freeIndexForScan = 0 s.allocCache = ^uint64(0) // all 1s indicating all free. s.gcmarkBits = newMarkBits(s.nelems) s.allocBits = newAllocBits(s.nelems) @@ -1565,6 +1574,7 @@ func (span *mspan) init(base uintptr, npages uintptr) { span.specials = nil span.needzero = 0 span.freeindex = 0 + span.freeIndexForScan = 0 span.allocBits = nil span.gcmarkBits = nil span.state.set(mSpanDead) -- cgit v1.2.1 From e727f41930ae9a8bedfbc85eb8ca7268486571ec Mon Sep 17 00:00:00 2001 From: Dmitri Goutnik Date: Fri, 20 May 2022 08:07:03 -0500 Subject: [release-branch.go1.18] cmd/cgo: recognize clang 14 DWARF type names Fixes #57028 Updates #53013 Change-Id: I169d4eb2420a6da52cc9abe17da98c3092a91be6 Reviewed-on: https://go-review.googlesource.com/c/go/+/407514 Auto-Submit: Ian Lance Taylor Reviewed-by: Cherry Mui Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gopher Robot (cherry picked from commit 2cfbef438049fd4c3f73d1562773ad1f93900897) Reviewed-on: https://go-review.googlesource.com/c/go/+/454415 Reviewed-by: Dmitri Shuralyov Run-TryBot: Heschi Kreinick Reviewed-by: Dmitri Shuralyov --- src/cmd/cgo/gcc.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 997a830994..dc5639812a 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -2215,6 +2215,8 @@ var dwarfToName = map[string]string{ "long long unsigned int": "ulonglong", "signed char": "schar", "unsigned char": "uchar", + "unsigned long": "ulong", // Used by Clang 14; issue 53013. + "unsigned long long": "ulonglong", // Used by Clang 14; issue 53013. } const signedDelta = 64 -- cgit v1.2.1 From 7013a4f5f816af62033ad63dd06b77c30d7a62a7 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 10 Nov 2022 12:16:27 -0800 Subject: [release-branch.go1.18] os, net/http: avoid escapes from os.DirFS and http.Dir on Windows Do not permit access to Windows reserved device names (NUL, COM1, etc.) via os.DirFS and http.Dir filesystems. Avoid escapes from os.DirFS(`\`) on Windows. DirFS would join the the root to the relative path with a path separator, making os.DirFS(`\`).Open(`/foo/bar`) open the path `\\foo\bar`, which is a UNC name. Not only does this not open the intended file, but permits reference to any file on the system rather than only files on the current drive. Make os.DirFS("") invalid, with all file access failing. Previously, a root of "" was interpreted as "/", which is surprising and probably unintentional. Fixes CVE-2022-41720. Fixes #56694. Change-Id: I275b5fa391e6ad7404309ea98ccc97405942e0f0 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1663832 Reviewed-by: Julie Qiu Reviewed-by: Tatiana Bradley Reviewed-on: https://go-review.googlesource.com/c/go/+/455360 Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Jenny Rakoczy --- src/go/build/deps_test.go | 1 + src/internal/safefilepath/path.go | 21 +++++++ src/internal/safefilepath/path_other.go | 23 ++++++++ src/internal/safefilepath/path_test.go | 88 ++++++++++++++++++++++++++++ src/internal/safefilepath/path_windows.go | 95 +++++++++++++++++++++++++++++++ src/net/http/fs.go | 8 ++- src/net/http/fs_test.go | 28 +++++++++ src/os/file.go | 36 +++++++++--- src/os/os_test.go | 38 +++++++++++++ 9 files changed, 328 insertions(+), 10 deletions(-) create mode 100644 src/internal/safefilepath/path.go create mode 100644 src/internal/safefilepath/path_other.go create mode 100644 src/internal/safefilepath/path_test.go create mode 100644 src/internal/safefilepath/path_windows.go diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 72465659dc..20a3d0fb86 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -168,6 +168,7 @@ var depsRules = ` io/fs < internal/testlog < internal/poll + < internal/safefilepath < os < os/signal; diff --git a/src/internal/safefilepath/path.go b/src/internal/safefilepath/path.go new file mode 100644 index 0000000000..0f0a270c30 --- /dev/null +++ b/src/internal/safefilepath/path.go @@ -0,0 +1,21 @@ +// Copyright 2022 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 safefilepath manipulates operating-system file paths. +package safefilepath + +import ( + "errors" +) + +var errInvalidPath = errors.New("invalid path") + +// FromFS converts a slash-separated path into an operating-system path. +// +// FromFS returns an error if the path cannot be represented by the operating +// system. For example, paths containing '\' and ':' characters are rejected +// on Windows. +func FromFS(path string) (string, error) { + return fromFS(path) +} diff --git a/src/internal/safefilepath/path_other.go b/src/internal/safefilepath/path_other.go new file mode 100644 index 0000000000..f93da18680 --- /dev/null +++ b/src/internal/safefilepath/path_other.go @@ -0,0 +1,23 @@ +// Copyright 2022 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. + +//go:build !windows + +package safefilepath + +import "runtime" + +func fromFS(path string) (string, error) { + if runtime.GOOS == "plan9" { + if len(path) > 0 && path[0] == '#' { + return path, errInvalidPath + } + } + for i := range path { + if path[i] == 0 { + return "", errInvalidPath + } + } + return path, nil +} diff --git a/src/internal/safefilepath/path_test.go b/src/internal/safefilepath/path_test.go new file mode 100644 index 0000000000..dc662c18b3 --- /dev/null +++ b/src/internal/safefilepath/path_test.go @@ -0,0 +1,88 @@ +// Copyright 2022 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 safefilepath_test + +import ( + "internal/safefilepath" + "os" + "path/filepath" + "runtime" + "testing" +) + +type PathTest struct { + path, result string +} + +const invalid = "" + +var fspathtests = []PathTest{ + {".", "."}, + {"/a/b/c", "/a/b/c"}, + {"a\x00b", invalid}, +} + +var winreservedpathtests = []PathTest{ + {`a\b`, `a\b`}, + {`a:b`, `a:b`}, + {`a/b:c`, `a/b:c`}, + {`NUL`, `NUL`}, + {`./com1`, `./com1`}, + {`a/nul/b`, `a/nul/b`}, +} + +// Whether a reserved name with an extension is reserved or not varies by +// Windows version. +var winreservedextpathtests = []PathTest{ + {"nul.txt", "nul.txt"}, + {"a/nul.txt/b", "a/nul.txt/b"}, +} + +var plan9reservedpathtests = []PathTest{ + {`#c`, `#c`}, +} + +func TestFromFS(t *testing.T) { + switch runtime.GOOS { + case "windows": + if canWriteFile(t, "NUL") { + t.Errorf("can unexpectedly write a file named NUL on Windows") + } + if canWriteFile(t, "nul.txt") { + fspathtests = append(fspathtests, winreservedextpathtests...) + } else { + winreservedpathtests = append(winreservedpathtests, winreservedextpathtests...) + } + for i := range winreservedpathtests { + winreservedpathtests[i].result = invalid + } + for i := range fspathtests { + fspathtests[i].result = filepath.FromSlash(fspathtests[i].result) + } + case "plan9": + for i := range plan9reservedpathtests { + plan9reservedpathtests[i].result = invalid + } + } + tests := fspathtests + tests = append(tests, winreservedpathtests...) + tests = append(tests, plan9reservedpathtests...) + for _, test := range tests { + got, err := safefilepath.FromFS(test.path) + if (got == "") != (err != nil) { + t.Errorf(`FromFS(%q) = %q, %v; want "" only if err != nil`, test.path, got, err) + } + if got != test.result { + t.Errorf("FromFS(%q) = %q, %v; want %q", test.path, got, err, test.result) + } + } +} + +func canWriteFile(t *testing.T, name string) bool { + path := filepath.Join(t.TempDir(), name) + os.WriteFile(path, []byte("ok"), 0666) + b, _ := os.ReadFile(path) + return string(b) == "ok" +} diff --git a/src/internal/safefilepath/path_windows.go b/src/internal/safefilepath/path_windows.go new file mode 100644 index 0000000000..909c150edc --- /dev/null +++ b/src/internal/safefilepath/path_windows.go @@ -0,0 +1,95 @@ +// Copyright 2022 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 safefilepath + +import ( + "syscall" + "unicode/utf8" +) + +func fromFS(path string) (string, error) { + if !utf8.ValidString(path) { + return "", errInvalidPath + } + for len(path) > 1 && path[0] == '/' && path[1] == '/' { + path = path[1:] + } + containsSlash := false + for p := path; p != ""; { + // Find the next path element. + i := 0 + dot := -1 + for i < len(p) && p[i] != '/' { + switch p[i] { + case 0, '\\', ':': + return "", errInvalidPath + case '.': + if dot < 0 { + dot = i + } + } + i++ + } + part := p[:i] + if i < len(p) { + containsSlash = true + p = p[i+1:] + } else { + p = "" + } + // Trim the extension and look for a reserved name. + base := part + if dot >= 0 { + base = part[:dot] + } + if isReservedName(base) { + if dot < 0 { + return "", errInvalidPath + } + // The path element is a reserved name with an extension. + // Some Windows versions consider this a reserved name, + // while others do not. Use FullPath to see if the name is + // reserved. + if p, _ := syscall.FullPath(part); len(p) >= 4 && p[:4] == `\\.\` { + return "", errInvalidPath + } + } + } + if containsSlash { + // We can't depend on strings, so substitute \ for / manually. + buf := []byte(path) + for i, b := range buf { + if b == '/' { + buf[i] = '\\' + } + } + path = string(buf) + } + return path, nil +} + +// isReservedName reports if name is a Windows reserved device name. +// It does not detect names with an extension, which are also reserved on some Windows versions. +// +// For details, search for PRN in +// https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file. +func isReservedName(name string) bool { + if 3 <= len(name) && len(name) <= 4 { + switch string([]byte{toUpper(name[0]), toUpper(name[1]), toUpper(name[2])}) { + case "CON", "PRN", "AUX", "NUL": + return len(name) == 3 + case "COM", "LPT": + return len(name) == 4 && '1' <= name[3] && name[3] <= '9' + } + } + return false +} + +func toUpper(c byte) byte { + if 'a' <= c && c <= 'z' { + return c - ('a' - 'A') + } + return c +} diff --git a/src/net/http/fs.go b/src/net/http/fs.go index 6caee9ed93..583203043f 100644 --- a/src/net/http/fs.go +++ b/src/net/http/fs.go @@ -9,6 +9,7 @@ package http import ( "errors" "fmt" + "internal/safefilepath" "io" "io/fs" "mime" @@ -69,14 +70,15 @@ func mapOpenError(originalErr error, name string, sep rune, stat func(string) (f // Open implements FileSystem using os.Open, opening files for reading rooted // and relative to the directory d. func (d Dir) Open(name string) (File, error) { - if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) { - return nil, errors.New("http: invalid character in file path") + path, err := safefilepath.FromFS(path.Clean("/" + name)) + if err != nil { + return nil, errors.New("http: invalid or unsafe file path") } dir := string(d) if dir == "" { dir = "." } - fullName := filepath.Join(dir, filepath.FromSlash(path.Clean("/"+name))) + fullName := filepath.Join(dir, path) f, err := os.Open(fullName) if err != nil { return nil, mapOpenError(err, fullName, filepath.Separator, os.Stat) diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go index d627dfd4be..323360d550 100644 --- a/src/net/http/fs_test.go +++ b/src/net/http/fs_test.go @@ -648,6 +648,34 @@ func TestFileServerZeroByte(t *testing.T) { } } +func TestFileServerNamesEscape(t *testing.T) { + t.Run("h1", func(t *testing.T) { + testFileServerNamesEscape(t, h1Mode) + }) + t.Run("h2", func(t *testing.T) { + testFileServerNamesEscape(t, h2Mode) + }) +} +func testFileServerNamesEscape(t *testing.T, h2 bool) { + defer afterTest(t) + ts := newClientServerTest(t, h2, FileServer(Dir("testdata"))).ts + defer ts.Close() + for _, path := range []string{ + "/../testdata/file", + "/NUL", // don't read from device files on Windows + } { + res, err := ts.Client().Get(ts.URL + path) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + if res.StatusCode < 400 || res.StatusCode > 599 { + t.Errorf("Get(%q): got status %v, want 4xx or 5xx", path, res.StatusCode) + } + + } +} + type fakeFileInfo struct { dir bool basename string diff --git a/src/os/file.go b/src/os/file.go index 2823128554..8afc6e3540 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -37,12 +37,12 @@ // Note: The maximum number of concurrent operations on a File may be limited by // the OS or the system. The number should be high, but exceeding it may degrade // performance or cause other issues. -// package os import ( "errors" "internal/poll" + "internal/safefilepath" "internal/testlog" "internal/unsafeheader" "io" @@ -623,6 +623,8 @@ func isWindowsNulName(name string) bool { // the /prefix tree, then using DirFS does not stop the access any more than using // os.Open does. DirFS is therefore not a general substitute for a chroot-style security // mechanism when the directory tree contains arbitrary content. +// +// The directory dir must not be "". func DirFS(dir string) fs.FS { return dirFS(dir) } @@ -641,10 +643,11 @@ func containsAny(s, chars string) bool { type dirFS string func (dir dirFS) Open(name string) (fs.File, error) { - if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) { - return nil, &PathError{Op: "open", Path: name, Err: ErrInvalid} + fullname, err := dir.join(name) + if err != nil { + return nil, &PathError{Op: "stat", Path: name, Err: err} } - f, err := Open(string(dir) + "/" + name) + f, err := Open(fullname) if err != nil { return nil, err // nil fs.File } @@ -652,16 +655,35 @@ func (dir dirFS) Open(name string) (fs.File, error) { } func (dir dirFS) Stat(name string) (fs.FileInfo, error) { - if !fs.ValidPath(name) || runtime.GOOS == "windows" && containsAny(name, `\:`) { - return nil, &PathError{Op: "stat", Path: name, Err: ErrInvalid} + fullname, err := dir.join(name) + if err != nil { + return nil, &PathError{Op: "stat", Path: name, Err: err} } - f, err := Stat(string(dir) + "/" + name) + f, err := Stat(fullname) if err != nil { return nil, err } return f, nil } +// join returns the path for name in dir. +func (dir dirFS) join(name string) (string, error) { + if dir == "" { + return "", errors.New("os: DirFS with empty root") + } + if !fs.ValidPath(name) { + return "", ErrInvalid + } + name, err := safefilepath.FromFS(name) + if err != nil { + return "", ErrInvalid + } + if IsPathSeparator(dir[len(dir)-1]) { + return string(dir) + name, nil + } + return string(dir) + string(PathSeparator) + name, nil +} + // ReadFile reads the named file and returns the contents. // A successful call returns err == nil, not err == EOF. // Because ReadFile reads the whole file, it does not treat an EOF from Read diff --git a/src/os/os_test.go b/src/os/os_test.go index 63427deb6e..4124be13cc 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -2696,6 +2696,44 @@ func TestDirFS(t *testing.T) { if err == nil { t.Fatalf(`Open testdata\dirfs succeeded`) } + + // Test that Open does not open Windows device files. + _, err = d.Open(`NUL`) + if err == nil { + t.Errorf(`Open NUL succeeded`) + } +} + +func TestDirFSRootDir(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + cwd = cwd[len(filepath.VolumeName(cwd)):] // trim volume prefix (C:) on Windows + cwd = filepath.ToSlash(cwd) // convert \ to / + cwd = strings.TrimPrefix(cwd, "/") // trim leading / + + // Test that Open can open a path starting at /. + d := DirFS("/") + f, err := d.Open(cwd + "/testdata/dirfs/a") + if err != nil { + t.Fatal(err) + } + f.Close() +} + +func TestDirFSEmptyDir(t *testing.T) { + d := DirFS("") + cwd, _ := os.Getwd() + for _, path := range []string{ + "testdata/dirfs/a", // not DirFS(".") + filepath.ToSlash(cwd) + "/testdata/dirfs/a", // not DirFS("/") + } { + _, err := d.Open(path) + if err == nil { + t.Fatalf(`DirFS("").Open(%q) succeeded`, path) + } + } } func TestDirFSPathsValid(t *testing.T) { -- cgit v1.2.1 From 76cad4edc29d28432a7a0aa27e87385d3d7db7a1 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 30 Nov 2022 16:37:07 -0500 Subject: [release-branch.go1.18] net/http: update bundled golang.org/x/net/http2 Disable cmd/internal/moddeps test, since this update includes PRIVATE track fixes. For #56350 For #57008 Fixes CVE-2022-41717 Change-Id: I31ebd2b9ae190ef6f7646187103ea1c8a713ff2e Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1663833 Reviewed-by: Tatiana Bradley Reviewed-by: Julie Qiu Reviewed-on: https://go-review.googlesource.com/c/go/+/455361 Run-TryBot: Jenny Rakoczy Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot --- src/cmd/internal/moddeps/moddeps_test.go | 2 ++ src/net/http/h2_bundle.go | 31 ++++++++++++++++++------------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/cmd/internal/moddeps/moddeps_test.go b/src/cmd/internal/moddeps/moddeps_test.go index 56c3b2585c..cd4d523e46 100644 --- a/src/cmd/internal/moddeps/moddeps_test.go +++ b/src/cmd/internal/moddeps/moddeps_test.go @@ -34,6 +34,8 @@ import ( // See issues 36852, 41409, and 43687. // (Also see golang.org/issue/27348.) func TestAllDependencies(t *testing.T) { + t.Skip("TODO(#57008): 1.18.9 contains unreleased changes from vendored modules") + goBin := testenv.GoToolPath(t) // Ensure that all packages imported within GOROOT diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 292dded28d..381f91c8ad 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -3384,10 +3384,11 @@ func (s http2SettingID) String() string { // name (key). See httpguts.ValidHeaderName for the base rules. // // Further, http2 says: -// "Just as in HTTP/1.x, header field names are strings of ASCII -// characters that are compared in a case-insensitive -// fashion. However, header field names MUST be converted to -// lowercase prior to their encoding in HTTP/2. " +// +// "Just as in HTTP/1.x, header field names are strings of ASCII +// characters that are compared in a case-insensitive +// fashion. However, header field names MUST be converted to +// lowercase prior to their encoding in HTTP/2. " func http2validWireHeaderFieldName(v string) bool { if len(v) == 0 { return false @@ -3578,8 +3579,8 @@ func (s *http2sorter) SortStrings(ss []string) { // validPseudoPath reports whether v is a valid :path pseudo-header // value. It must be either: // -// *) a non-empty string starting with '/' -// *) the string '*', for OPTIONS requests. +// *) a non-empty string starting with '/' +// *) the string '*', for OPTIONS requests. // // For now this is only used a quick check for deciding when to clean // up Opaque URLs before sending requests from the Transport. @@ -4242,6 +4243,7 @@ type http2serverConn struct { headerTableSize uint32 peerMaxHeaderListSize uint32 // zero means unknown (default) canonHeader map[string]string // http2-lower-case -> Go-Canonical-Case + canonHeaderKeysSize int // canonHeader keys size in bytes writingFrame bool // started writing a frame (on serve goroutine or separate) writingFrameAsync bool // started a frame on its own goroutine but haven't heard back on wroteFrameCh needsFrameFlush bool // last frame write wasn't a flush @@ -4421,6 +4423,13 @@ func (sc *http2serverConn) condlogf(err error, format string, args ...interface{ } } +// maxCachedCanonicalHeadersKeysSize is an arbitrarily-chosen limit on the size +// of the entries in the canonHeader cache. +// This should be larger than the size of unique, uncommon header keys likely to +// be sent by the peer, while not so high as to permit unreasonable memory usage +// if the peer sends an unbounded number of unique header keys. +const http2maxCachedCanonicalHeadersKeysSize = 2048 + func (sc *http2serverConn) canonicalHeader(v string) string { sc.serveG.check() http2buildCommonHeaderMapsOnce() @@ -4436,14 +4445,10 @@ func (sc *http2serverConn) canonicalHeader(v string) string { sc.canonHeader = make(map[string]string) } cv = CanonicalHeaderKey(v) - // maxCachedCanonicalHeaders is an arbitrarily-chosen limit on the number of - // entries in the canonHeader cache. This should be larger than the number - // of unique, uncommon header keys likely to be sent by the peer, while not - // so high as to permit unreaasonable memory usage if the peer sends an unbounded - // number of unique header keys. - const maxCachedCanonicalHeaders = 32 - if len(sc.canonHeader) < maxCachedCanonicalHeaders { + size := 100 + len(v)*2 // 100 bytes of map overhead + key + value + if sc.canonHeaderKeysSize+size <= http2maxCachedCanonicalHeadersKeysSize { sc.canonHeader[v] = cv + sc.canonHeaderKeysSize += size } return cv } -- cgit v1.2.1 From 0d8a92bdfd3d6d1b24f47e05f9be46645aec94f0 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Tue, 6 Dec 2022 19:01:34 +0000 Subject: [release-branch.go1.18] go1.18.9 Change-Id: Ida61e740fc342357f54e523c0044cf6d83e0baec Reviewed-on: https://go-review.googlesource.com/c/go/+/455597 TryBot-Result: Gopher Robot Reviewed-by: Michael Pratt Reviewed-by: Jenny Rakoczy Run-TryBot: Gopher Robot Auto-Submit: Gopher Robot --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index aea94a5de8..a7cd7b8f21 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -go1.18.8 \ No newline at end of file +go1.18.9 \ No newline at end of file -- cgit v1.2.1