Filippo Valsorda [Fri, 26 Jan 2024 22:22:45 +0000 (23:22 +0100)]
[release-branch.go1.20] Revert "crypto/internal/boring: upgrade module to fips-20220613" +1
This reverts CL 553875 ("crypto/internal/boring: upgrade module to
fips-20220613") and CL 553876 ("crypto/tls: align FIPS-only mode with
BoringSSL policy").
Michael Pratt [Wed, 13 Dec 2023 20:07:28 +0000 (15:07 -0500)]
[release-branch.go1.20] runtime: properly model rwmutex in lock ranking
(This cherry-pick combines CL 549536 and the follow-up fix CL 555055.)
Currently, lock ranking doesn't really try to model rwmutex. It records
the internal locks rLock and wLock, but in a subpar fashion:
1. wLock is held from lock to unlock, so it works OK, but it conflates
write locks of all rwmutexes as rwmutexW, rather than allowing
different rwmutexes to have different rankings.
2. rLock is an internal implementation detail that is only taken when
there is contention in rlock. As as result, the reader lock path is
almost never checked.
Add proper modeling. rwmutexR and rwmutexW remain as the ranks of the
internal locks, which have their own ordering. The new init method is
passed the ranks of the higher level lock that this represents, just
like lockInit for mutex.
execW ordered before MALLOC captures the case from #64722. i.e., there
can be allocation between BeforeFork and AfterFork.
For #64722.
Fixes #64760.
------
runtime: replace rwmutexR/W with per-rwmutex lock rank
CL 549536 intended to decouple the internal implementation of rwmutex
from the semantic meaning of an rwmutex read/write lock in the static
lock ranking.
Unfortunately, it was not thought through well enough. The internals
were represented with the rwmutexR and rwmutexW lock ranks. The idea was
that the internal lock ranks need not model the higher-level ordering,
since those have separate rankings. That is incorrect; rwmutexW is held
for the duration of a write lock, so it must be ranked before any lock
taken while any write lock is held, which is precisely what we were
trying to avoid.
This is visible in violations like:
0 : execW 11 0x0
1 : rwmutexW 51 0x111d9c8
2 : fin 30 0x111d3a0
fatal error: lock ordering problem
execW < fin is modeled, but rwmutexW < fin is missing.
Fix this by eliminating the rwmutexR/W lock ranks shared across
different types of rwmutex. Instead require users to define an
additional "internal" lock rank to represent the implementation details
of rwmutex.rLock. We can avoid an additional "internal" lock rank for
rwmutex.wLock because the existing writeRank has the same semantics for
semantic and internal locking. i.e., writeRank is held for the duration
of a write lock, which is exactly how rwmutex.wLock is used, so we can
use writeRank directly on wLock.
For #64722.
Cq-Include-Trybots: luci.golang.try:go1.20-linux-amd64-staticlockranking
Change-Id: I23335b28faa42fb04f1bc9da02fdf54d1616cd28
Reviewed-on: https://go-review.googlesource.com/c/go/+/549536 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 9b4b3e5acca2dabe107fa2c3ed963097d78a4562)
(cherry picked from commit dcbe77246922fe7ef41f07df228f47a37803f360)
Reviewed-on: https://go-review.googlesource.com/c/go/+/554995
Michael Anthony Knyszek [Mon, 27 Nov 2023 22:27:32 +0000 (22:27 +0000)]
[release-branch.go1.20] runtime: put ReadMemStats debug assertions behind a double-check mode
ReadMemStats has a few assertions it makes about the consistency of the
stats it's about to produce. Specifically, how those stats line up with
runtime-internal stats. These checks are generally useful, but crashing
just because some stats are wrong is a heavy price to pay.
For a long time this wasn't a problem, but very recently it became a
real problem. It turns out that there's real benign skew that can happen
wherein sysmon (which doesn't synchronize with a STW) generates a trace
event when tracing is enabled, and may mutate some stats while
ReadMemStats is running its checks.
Fix this by synchronizing with both sysmon and the tracer. This is a bit
heavy-handed, but better that than false positives.
Also, put the checks behind a debug mode. We want to reduce the risk of
backporting this change, and again, it's not great to crash just because
user-facing stats are off. Still, enable this debug mode during the
runtime tests so we don't lose quite as much coverage from disabling
these checks by default.
For #64401.
Fixes #64409.
Change-Id: I9adb3e5c7161d207648d07373a11da8a5f0fda9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/545277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
(cherry picked from commit b2efd1de97402ec4b8fb4e9e0ec29c8e49e8e200)
Reviewed-on: https://go-review.googlesource.com/c/go/+/545556
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Michael Pratt [Wed, 1 Nov 2023 21:06:08 +0000 (17:06 -0400)]
[release-branch.go1.20] os/signal: remove go t.Run from TestNohup
Since CL 226138, TestNohup has a bit of a strange construction: it wants
to run the "uncaught" subtests in parallel with each other, and the
"nohup" subtests in parallel with each other, but also needs join
between "uncaught" and "nohop" so it can Stop notifying for SIGHUP.
It achieves this by doing `go t.Run` with a WaitGroup rather than using
`t.Parallel` in the subtest (which would make `t.Run` return immediately).
However, this makes things more difficult to understand than necessary.
As noted on https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks,
a second layer of subtest can be used to join parallel subtests.
Switch to this form, which makes the test simpler to follow
(particularly the cleanup that goes with "uncaught").
For #63799.
For #63910.
Change-Id: Ibfce0f439508a7cfca848c7ccfd136c9c453ad8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/538899
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit 5622a4b2054664edcdd64974b9df73b440aedfae)
Reviewed-on: https://go-review.googlesource.com/c/go/+/546375
Damien Neil [Tue, 7 Nov 2023 18:47:56 +0000 (10:47 -0800)]
[release-branch.go1.20] net/http: limit chunked data overhead
The chunked transfer encoding adds some overhead to
the content transferred. When writing one byte per
chunk, for example, there are five bytes of overhead
per byte of data transferred: "1\r\nX\r\n" to send "X".
Chunks may include "chunk extensions",
which we skip over and do not use.
For example: "1;chunk extension here\r\nX\r\n".
A malicious sender can use chunk extensions to add
about 4k of overhead per byte of data.
(The maximum chunk header line size we will accept.)
Track the amount of overhead read in chunked data,
and produce an error if it seems excessive.
Updates #64433
Fixes #64434
Fixes CVE-2023-39326
Change-Id: I40f8d70eb6f9575fb43f506eb19132ccedafcf39
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2076135 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit 3473ae72ee66c60744665a24b2fde143e8964d4f)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2095407
Run-TryBot: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/547355 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Jorropo [Sun, 5 Nov 2023 21:40:01 +0000 (22:40 +0100)]
[release-branch.go1.20] cmd/compile: fix findIndVar so it does not match disjointed loop headers
Fix #63983
parseIndVar, prove and maybe more are on the assumption that the loop header
is a single block. This can be wrong, ensure we don't match theses cases we
don't know how to handle.
In the future we could update them so that they know how to handle such cases
but theses cases seems rare so I don't think the value would be really high.
We could also run a loop canonicalization pass first which could handle this.
The repro case looks weird because I massaged it so it would crash with the
previous compiler.
Change-Id: I4aa8afae9e90a17fa1085832250fc1139c97faa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/539977 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 8b4e1259d0e82c8fe38a1456f997a4e9d63573a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/539936
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Jorropo <jorropo.pgm@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Roland Shoemaker [Wed, 18 Oct 2023 15:25:08 +0000 (11:25 -0400)]
[release-branch.go1.20] crypto/rand,runtime: switch RtlGenRandom for ProcessPrng
RtlGenRandom is a semi-undocumented API, also known as
SystemFunction036, which we use to generate random data on Windows.
It's definition, in cryptbase.dll, is an opaque wrapper for the
documented API ProcessPrng. Instead of using RtlGenRandom, switch to
using ProcessPrng, since the former is simply a wrapper for the latter,
there should be no practical change on the user side, other than a minor
change in the DLLs we load.
Updates #53192
Fixes #64412
Change-Id: Ie6891bf97b1d47f5368cccbe92f374dba2c2672a
Reviewed-on: https://go-review.googlesource.com/c/go/+/536235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 693def151adff1af707d82d28f55dba81ceb08e1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/545356
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Damien Neil [Thu, 9 Nov 2023 17:53:44 +0000 (09:53 -0800)]
[release-branch.go1.20] path/filepath: consider \\?\c: as a volume on Windows
While fixing several bugs in path handling on Windows,
beginning with \\?\.
Prior to #540277, VolumeName considered the first path component
after the \\?\ prefix to be part of the volume name.
After, it considered only the \\? prefix to be the volume name.
Restore the previous behavior.
For #64028.
Fixes #64040.
Change-Id: I6523789e61776342800bd607fb3f29d496257e68
Reviewed-on: https://go-review.googlesource.com/c/go/+/541175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
(cherry picked from commit eda42f7c60adab26ed1a340414c726c4bf46b1f7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/541520
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
[release-branch.go1.20] cmd/go/internal/modfetch/codehost: set core.longpaths in Git repos on Windows
This setting appears to be needed to avoid “Filename too long” errors
when downloading modules from repos with long branch names,
particularly if the path to the module cache is already fairly long
(as may be the case in CI systems and in tests of cmd/go itself).
Damien Neil [Fri, 1 Sep 2023 18:17:19 +0000 (11:17 -0700)]
[release-branch.go1.20] path/filepath: fix various issues in parsing Windows paths
On Windows, A root local device path is a path which begins with
\\?\ or \??\. A root local device path accesses the DosDevices
object directory, and permits access to any file or device on the
system. For example \??\C:\foo is equivalent to common C:\foo.
The Clean, IsAbs, IsLocal, and VolumeName functions did not
recognize root local device paths beginning with \??\.
Clean could convert a rooted path such as \a\..\??\b into
the root local device path \??\b. It will now convert this
path into .\??\b.
IsAbs now correctly reports paths beginning with \??\
as absolute.
IsLocal now correctly reports paths beginning with \??\
as non-local.
VolumeName now reports the \??\ prefix as a volume name.
Join(`\`, `??`, `b`) could convert a seemingly innocent
sequence of path elements into the root local device path
\??\b. It will now convert this to \.\??\b.
In addition, the IsLocal function did not correctly
detect reserved names in some cases:
- reserved names followed by spaces, such as "COM1 ".
- "COM" or "LPT" followed by a superscript 1, 2, or 3.
IsLocal now correctly reports these names as non-local.
For #63713
Fixes #63714
Fixes CVE-2023-45283
Fixes CVE-2023-45284
Change-Id: I446674a58977adfa54de7267d716ac23ab496c54
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2040691 Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2072597 Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/539276
Auto-Submit: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Than McIntosh [Fri, 29 Sep 2023 18:19:17 +0000 (14:19 -0400)]
[release-branch.go1.20] cmd/link: split text sections for arm 32-bit
This CL is a roll-forward (tweaked slightly) of CL 467715, which
turned on text section splitting for GOARCH=arm. The intent is to
avoid recurrent problems with external linking where there is a
disagreement between the Go linker and the external linker over
whether a given branch will reach. In the past our approach has been
to tweak the reachability calculations slightly to try to work around
potential linker problems, but this hasn't proven to be very robust;
section splitting seems to offer a better long term fix.
Updates #58425.
Fixes #63316.
Change-Id: I7372d41abce84097906a3d0805b6b9c486f345d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/531795 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 1e690409206ff97330b5a91517d453fc5129bab2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/532097
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Dmitri Shuralyov [Tue, 10 Oct 2023 17:27:58 +0000 (13:27 -0400)]
[release-branch.go1.20] all: tidy dependency versioning after release
Done with:
go get golang.org/x/net@internal-branch.go1.20-vendor
go mod tidy
go mod vendor
go generate net/http # zero diff since CL 534255 already did this
For #63417.
For #63426.
For CVE-2023-39325.
Change-Id: Ib258e0d8165760a1082e02c2f4c5ce7d2a3c3c90
Reviewed-on: https://go-review.googlesource.com/c/go/+/534297
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Ian Lance Taylor [Wed, 20 Sep 2023 23:16:29 +0000 (16:16 -0700)]
[release-branch.go1.20] cmd/compile: use absolute file name in isCgo check
For #23672
Updates #63211
Fixes #63213
Fixes CVE-2023-39323
Change-Id: I4586a69e1b2560036afec29d53e53cf25e6c7352
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2032884 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit 9b19e751918dd218035811b1ef83a8c2693b864a)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2037629 Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/533195
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Cherry Mui [Mon, 10 Jul 2023 22:50:06 +0000 (18:50 -0400)]
[release-branch.go1.20] cmd/link: suppress -bind_at_load deprecation warning for ld-prime
ld-prime emits a deprecation warning for -bind_at_load. The flag
is needed for plugins to not deadlock (#38824) when linking with
older darwin linker. It is supposedly not needed with newer linker
when chained fixups are used. For now, we always pass it, and
suppress the warning.
Updates #61229.
For #62597.
Change-Id: I4b8a6f864a460c40dc38adbb533f664f7fd5343c
Reviewed-on: https://go-review.googlesource.com/c/go/+/508696 Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 040dbf9c181a0e3ea9f7bd3ebe3f75acdc878aaf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/527798
Cherry Mui [Tue, 12 Sep 2023 19:40:09 +0000 (15:40 -0400)]
[release-branch.go1.20] cmd/link: force old Apple linker in plugin mode
There are some bugs in Apple's new linker that probably will not
be fixed when Xcode 15 is released (some time soon). We fix/work
around them but it is too much to backport them all. Force old
Apple linker to work around.
Updates #61229.
For #62597.
Change-Id: Ia5941918e882b22b4dbc41c74764d19d413d0b56
Reviewed-on: https://go-review.googlesource.com/c/go/+/527818
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Roland Shoemaker [Thu, 3 Aug 2023 19:28:28 +0000 (12:28 -0700)]
[release-branch.go1.20] html/template: properly handle special tags within the script context
The HTML specification has incredibly complex rules for how to handle
"<!--", "<script", and "</script" when they appear within literals in
the script context. Rather than attempting to apply these restrictions
(which require a significantly more complex state machine) we apply
the workaround suggested in section 4.12.1.3 of the HTML specification [1].
More precisely, when "<!--", "<script", and "</script" appear within
literals (strings and regular expressions, ignoring comments since we
already elide their content) we replace the "<" with "\x3C". This avoids
the unintuitive behavior that using these tags within literals can cause,
by simply preventing the rendered content from triggering it. This may
break some correct usages of these tags, but on balance is more likely
to prevent XSS attacks where users are unknowingly either closing or not
closing the script blocks where they think they are.
Thanks to Takeshi Kaneko (GMO Cybersecurity by Ierae, Inc.) for
reporting this issue.
Roland Shoemaker [Thu, 3 Aug 2023 19:24:13 +0000 (12:24 -0700)]
[release-branch.go1.20] html/template: support HTML-like comments in script contexts
Per Appendix B.1.1 of the ECMAScript specification, support HTML-like
comments in script contexts. Also per section 12.5, support hashbang
comments. This brings our parsing in-line with how browsers treat these
comment types.
Thanks to Takeshi Kaneko (GMO Cybersecurity by Ierae, Inc.) for
reporting this issue.
Fixes #62196
Fixes #62395
Fixes CVE-2023-39318
Change-Id: Id512702c5de3ae46cf648e268cb10e1eb392a181
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1976593
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2014620
Reviewed-on: https://go-review.googlesource.com/c/go/+/526098
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
David Chase [Tue, 15 Aug 2023 21:45:50 +0000 (17:45 -0400)]
[release-branch.go1.20] cmd/compile: in expandCalls, move all arg marshalling into call block
For aggregate-typed arguments passed to a call, expandCalls
decomposed them into parts in the same block where the value
was created. This is not necessarily the call block, and in
the case where stores are involved, can change the memory
leaving that block, and getting that right is problematic.
Instead, do all the expanding in the same block as the call,
which avoids the problems of (1) not being able to reorder
loads/stores across a block boundary to conform to memory
order and (2) (incorrectly, not) exposing the new memory to
consumers in other blocks. Putting it all in the same block
as the call allows reordering, and the call creates its own
new memory (which is already dealt with correctly).
Fixes #62056.
Updates #61992.
Change-Id: Icc7918f0d2dd3c480cc7f496cdcd78edeca7f297
Reviewed-on: https://go-review.googlesource.com/c/go/+/519276 Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit e72ecc6a6becab9ca895c0108047db4723394296)
Reviewed-on: https://go-review.googlesource.com/c/go/+/520059
David Chase [Fri, 11 Aug 2023 18:38:19 +0000 (14:38 -0400)]
[release-branch.go1.20] runtime: profiling on Darwin cannot use blocking reads
On Darwin (and assume also on iOS but not sure), notetsleepg
cannot be called in a signal-handling context. Avoid this
by disabling block reads on Darwin.
An alternate approach was to add "sigNote" with a pipe-based
implementation on Darwin, but that ultimately would have required
at least one more linkname between runtime and syscall to avoid
racing with fork and opening the pipe, so, not.
Fixes #62018.
Updates #61768.
Change-Id: I0e8dd4abf9a606a3ff73fc37c3bd75f55924e07e
Reviewed-on: https://go-review.googlesource.com/c/go/+/518836
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit c6ee8e31e3e7b01da54d241ae5a8eed350ab0e9f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/518677
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Austin Clements <austin@google.com>
Russ Cox [Wed, 16 Aug 2023 15:18:02 +0000 (11:18 -0400)]
[release-branch.go1.20] cmd/api: rename api.go to main_test.go
This makes cmd/api no longer an importable package.
In CL 453258 I forgot that there was no direct prohibition
on importing packages from cmd - we just rely on the
fact that cmd/* is all package main and everything else
is cmd/internal.
Russ Cox [Fri, 16 Jun 2023 20:30:52 +0000 (16:30 -0400)]
[release-branch.go1.20] cmd/go: do not index std as a module in modcache
We do not index std as a whole module ever.
When working in the main Go repo, files in package change often,
so we don't want to pay the cost of reindexing all of std when what
we really need is just to reindex strings. Per-package indexing
works better for that case.
When using a released Go toolchain, we don't have to worry about
the whole module changing, but if we switch to whole-module indexing
at that point, we have the potential for bugs that only happen in
released toolchains. Probably not worth the risk.
For similar reasons, we don't index the current work module as
a whole module (individual packages are changing), so we use the heuristic
that we only do whole-module indexing in the module cache.
The new toolchain modules live in the module cache, though, and
our heuristic was causing whole-module indexing for them.
As predicted, enabling whole-module indexing for std when it's
completely untested does in fact lead to bugs (a very minor one).
This CL turns off whole-module indexing for std even when it is
in the module cache, to bring toolchain module behavior back in
line with the other ways to run toolchains.
Damien Neil [Wed, 19 Jul 2023 17:30:46 +0000 (10:30 -0700)]
[release-branch.go1.20] net/http: permit requests with invalid Host headers
Historically, the Transport has silently truncated invalid
Host headers at the first '/' or ' ' character. CL 506996 changed
this behavior to reject invalid Host headers entirely.
Unfortunately, Docker appears to rely on the previous behavior.
When sending a HTTP/1 request with an invalid Host, send an empty
Host header. This is safer than truncation: If you care about the
Host, then you should get the one you set; if you don't care,
then an empty Host should be fine.
Continue to fully validate Host headers sent to a proxy,
since proxies generally can't productively forward requests
without a Host.
For #60374
Fixes #61431
Fixes #61826
Change-Id: If170c7dd860aa20eb58fe32990fc93af832742b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/511155
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit b9153f6ef338baee5fe02a867c8fbc83a8b29dd1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/518756
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Russ Cox [Fri, 11 Aug 2023 14:58:26 +0000 (10:58 -0400)]
[release-branch.go1.20] cmd/go: refuse to build Go 1.22 code
With #60078 accepted, we expect Go 1.22 will have different
for loop semantics than Go 1.20 did. Once Go 1.22 is released,
Go 1.20 will be unsupported, but add a check anyway, just to
help catch some mistakes and usage of old Go toolchains
beyond their end-of-support.
Note that Go 1.20 can keep being used indefinitely with pre-Go 1.22 code.
This change only makes it refuse to build code that says it needs
Go 1.22 semantics, because Go 1.20 does not provide those.
Roland Shoemaker [Wed, 7 Jun 2023 22:27:13 +0000 (15:27 -0700)]
[release-branch.go1.20] crypto/tls: restrict RSA keys in certificates to <= 8192 bits
Extremely large RSA keys in certificate chains can cause a client/server
to expend significant CPU time verifying signatures. Limit this by
restricting the size of RSA keys transmitted during handshakes to <=
8192 bits.
Based on a survey of publicly trusted RSA keys, there are currently only
three certificates in circulation with keys larger than this, and all
three appear to be test certificates that are not actively deployed. It
is possible there are larger keys in use in private PKIs, but we target
the web PKI, so causing breakage here in the interests of increasing the
default safety of users of crypto/tls seems reasonable.
Thanks to Mateusz Poliwczak for reporting this issue.
Updates #61460
Fixes #61580
Fixes CVE-2023-29409
Change-Id: Ie35038515a649199a36a12fc2c5df3af855dca6c
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1912161 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit d865c715d92887361e4bd5596e19e513f27781b7)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1965747
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/514900
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
[release-branch.go1.20] cmd/asm, cmd/internal/obj: generate proper atomic ops for riscv64
Go's memory model closely follows the approach C++ concurrency memory
model (https://go.dev/ref/mem) and Go atomic "has the same semantics as C++'s
sequentially consistent atomics".
Meanwhile according to RISCV manual A.6 "Mappings from C/C++ primitives to RISC-V primitives".
C/C++ atomic operations (memory_order_acq_rel) should be map to "amo<op>.{w|d}.aqrl"
LR/SC (memory_order_acq_rel) should map to "lr.{w|d}.aq; <op>; sc.{w|d}.rl"
Damien Neil [Wed, 28 Jun 2023 20:20:08 +0000 (13:20 -0700)]
[release-branch.go1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996 Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/507357 Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Tatiana Bradley <tatianabradley@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Jelle van den Hooff [Thu, 22 Jun 2023 01:28:05 +0000 (18:28 -0700)]
[release-branch.go1.20] runtime: set raceignore to zero when starting a new goroutine
When reusing a g struct the runtime did not reset
g.raceignore. Initialize raceignore to zero when initially
setting racectx.
A goroutine can end with a non-zero raceignore if it exits
after calling runtime.RaceDisable without a matching
runtime.RaceEnable. If that goroutine's g is later reused
the race detector is in a weird state: the underlying
g.racectx is active, yet g.raceignore is non-zero, and
raceacquire/racerelease which check g.raceignore become
no-ops. This causes the race detector to report races when
there are none.
For #60934
Fixes #60949
Change-Id: Ib8e412f11badbaf69a480f03740da70891f4093f
Reviewed-on: https://go-review.googlesource.com/c/go/+/505055 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 48dbb6227acf3ebc8ac21924567aa2b6d5064915)
Reviewed-on: https://go-review.googlesource.com/c/go/+/505676 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Alex Brainman [Sat, 4 Mar 2023 03:35:35 +0000 (14:35 +1100)]
[release-branch.go1.20] runtime: allow for 5 more threads in TestWindowsStackMemory*
Original version of TestWindowsStackMemory did not consider sysmon and
other threads running during the test. Allow for 5 extra threads in this
test - this should cover any new threads in the future.
For #58570
Change-Id: I215790f9b94ff40a32ddd7aa54af715d1dc391c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/473415 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit f6cbc1da05da141a78ee33954b52d17642c95130)
Reviewed-on: https://go-review.googlesource.com/c/go/+/506975
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Roland Shoemaker [Wed, 21 Jun 2023 21:43:05 +0000 (14:43 -0700)]
[release-branch.go1.20] crypto/x509: tolerate multiple matching chains in testVerify
Due to the semantics of roots, a root store may contain two valid roots
that have the same subject (but different SPKIs) at the asme time. As
such in testVerify it is possible that when we verify a certificate we
may get two chains that has the same stringified representation.
Rather than doing something fancy to include keys (which is just overly
complicated), tolerate multiple matches.
Updates #60925
Fixes #60947
Change-Id: I5f51f7635801762865a536bcb20ec75f217a36ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/505035 Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 20313660f5f3a87dfd9074c4061c521fa25fcd32)
Reviewed-on: https://go-review.googlesource.com/c/go/+/505275
Run-TryBot: Heschi Kreinick <heschi@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
[release-branch.go1.20] runtime: resolve checkdead panic by refining `startm` lock handling in caller context
This change addresses a `checkdead` panic caused by a race condition between
`sysmon->startm` and `checkdead` callers, due to prematurely releasing the
scheduler lock. The solution involves allowing a `startm` caller to acquire the
scheduler lock and call `startm` in this context. A new `lockheld` bool
argument is added to `startm`, which manages all lock and unlock calls within
the function. The`startIdle` function variable in `injectglist` is updated to
call `startm` with the lock held, ensuring proper lock handling in this
specific case. This refined lock handling resolves the observed race condition
issue.
For #59600.
Fixes #60760.
Change-Id: I11663a15536c10c773fc2fde291d959099aa71be
Reviewed-on: https://go-review.googlesource.com/c/go/+/487316 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
(cherry picked from commit ff059add10d71fe13239cf893c0cca113de1fc21)
Reviewed-on: https://go-review.googlesource.com/c/go/+/504395 Reviewed-by: Lucien Coffe <lucien.coffe@botify.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
[release-branch.go1.20] runtime: fallback to TEB arbitrary pointer when TLS slots are full
Note: This CL is cherry-picked from CL 486816 omitting the changes
in sys_windows_386.s, which don't apply to go1.20 release branch
because windows/386 started using the TEB TLS slot in go1.21 (CL 454675).
The Go runtime allocates the TLS slot in the TEB TLS slots instead of
using the TEB arbitrary pointer. See CL 431775 for more context.
The problem is that the TEB TLS slots array only has capacity for 64
indices, allocating more requires some complex logic that we don't
support yet.
Although the Go runtime only allocates one index, a Go DLL can be
loaded in a process with more than 64 TLS slots allocated,
in which case it abort.
This CL avoids aborting by falling back to the older behavior, that
is to use the TEB arbitrary pointer.
Fixes #60535
Updates #59213
Change-Id: I39c73286fe2da95aa9c5ec5657ee0979ecbec533
Reviewed-on: https://go-review.googlesource.com/c/go/+/486816 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 715d53c090ea02dbd73c301684ecbd09b476989e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/504475 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Cherry Mui [Thu, 15 Jun 2023 22:16:51 +0000 (18:16 -0400)]
[release-branch.go1.20] runtime: use 1-byte load for address checking in racecallatomic
In racecallatomic, we do a load before calling into TSAN, so if
the address is invalid we fault on the Go stack. We currently use
a 8-byte load instruction, regardless of the data size that the
atomic operation is performed on. So if, say, we are doing a
LoadUint32 at an address that is the last 4 bytes of a memory
mapping, we may fault unexpectedly. Do a 1-byte load instead.
(Ideally we should do a load with the right size, so we fault
correctly if we're given an unaligned address for a wide load
across a page boundary. Leave that for another CL.)
Fix AMD64, ARM64, and PPC64. The code already uses 1-byte load
on S390X.
Fixes #60845.
Updates #60825.
Change-Id: I3dee93eb08ba180c85e86a9d2e71b5b520e8dcf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/503937
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 1a7709d6af76c06d465c5e969b502fc206f8e687)
Reviewed-on: https://go-review.googlesource.com/c/go/+/503976
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Austin Clements <austin@google.com>
Ian Lance Taylor [Mon, 19 Jun 2023 22:35:50 +0000 (15:35 -0700)]
[release-branch.go1.20] net/mail: permit more characters in mail headers
We parse mail messages using net/textproto. For #53188, we tightened
up the bytes permitted by net/textproto to match RFC 7230.
However, this package uses RFC 5322 which is more permissive.
Restore the permisiveness we used to have, so that older code
continues to work.
For #58862
For #60332
Fixes #60874
Fixes #60875
Change-Id: I5437f5e18a756f6ca61c13c4d8ba727be73eff9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/504881
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Heschi Kreinick [Wed, 29 Mar 2023 17:13:19 +0000 (13:13 -0400)]
[release-branch.go1.20] cmd/go: fix tests for new builder environment
Fix two long tests that fail in the builders we're trying out:
- TestQueryImport was failing with:
open /nonexist-gopath/pkg/sumdb/sum.golang.org/latest: no such file or directory
which eventually turns out to be because it couldn't create
/nonexist-gopath because it wasn't running as root. The test already
uses a temporary GOPATH, but missed overriding a configuration
variable set at init time.
- test_flags fails if the working directory has /x/ in it, which it now
happens to.
Bryan C. Mills [Mon, 15 May 2023 15:49:56 +0000 (11:49 -0400)]
[release-branch.go1.20] cmd/cover: error out if a requested source file contains a newline
cmd/cover uses '//line' directives to map instrumented source files
back to the original source file and line numbers.
Line directives have no way to escape newline characters, so cmd/cover
must not be used with source file paths that contain such characters.
Updates #60516.
Updates #60167.
Change-Id: I6dc039392d59fc3a5a6121ef6ca97b0ab0da5288
Reviewed-on: https://go-review.googlesource.com/c/go/+/501577
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 3d78c735fc7d213e23383b9744297bd5251dc0e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501818
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Cuong Manh Le [Thu, 8 Jun 2023 02:58:22 +0000 (09:58 +0700)]
[release-branch.go1.20] cmd/compile: do not report division by error during typecheck
types2 have already errored about any spec-required overflows, and
division by zero. CL 469595 unintentionally fixed typecheck not to error
about overflows, but zero division is still be checked during tcArith.
This causes unsafe operations with variable size failed to compile,
instead of raising runtime error.
This CL also making change to typecheck.EvalConst, to stop evaluating
literal shifts or binary operators where {over,under}flows can happen.
For go1.21, typecheck.EvalConst is removed entirely, but that change is
too large to backport.
See discussion in CL 501735 for more details.
Fixes #60675
Change-Id: I7bea2821099556835c920713397f7c5d8a4025ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/501735
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/503855
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Bryan C. Mills [Sat, 20 May 2023 02:35:33 +0000 (22:35 -0400)]
[release-branch.go1.20] cmd/go: retain extra roots to disambiguate imports in 'go mod tidy'
We don't normally keep explicit requirements for test dependencies of
packages loaded from other modules when the required version is
already the selected version in the module graph. However, in some
cases we may need to keep an explicit requirement in order to make use
of lazy module loading to disambiguate an otherwise-ambiguous import.
Note that there is no Go version guard for this change: in the cases
where the behavior of 'go mod tidy' has changed, previous versions of
Go would produce go.mod files that break successive calls to
'go mod tidy'. Given that, I suspect that any existing user in the
wild affected by this bug either already has a workaround in place
using redundant import statements (in which case the change does not
affect them) or is running 'go mod tidy -e' to force past the error
(in which case a change in behavior to a non-error should not be
surprising).
Before, if a hash was exactly 66 bytes long, we weren't truncating it
for use with P-521, because the byte length was not overflowing.
However, the bit length could still overflow.
Fixes #60744
Updates #60741
Change-Id: I37a0ee210add0eb566e6dc1c141e83e992983eb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/502478
Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 886fba5871268c2dccba2675fea5aafacab59189)
Reviewed-on: https://go-review.googlesource.com/c/go/+/502915
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Michael Matloob [Mon, 12 Jun 2023 16:33:30 +0000 (12:33 -0400)]
[release-branch.go1.20] go/build: check for invalid import paths again
The go parser previously checked for invalid import paths, go/build,
seeing the parse error would put files with invalid import paths into
InvalidGoFiles. golang.org/cl/424855 removed that check from the
parser, which meant files with invalid import paths not have any parse
errors on them and not be put into InvalidGoFiles. Do a check for
invalid import paths in go/build soon after parsing so we can make
sure files with invalid import paths go into InvalidGoFiles.
This fixes an issue where the Go command assumed that if a file wasn't
invalid it had non empty import paths, leading to a panic.
Fixes #60754
Updates #60230
Updates #60686
Change-Id: I33c1dc9304649536834939cef7c689940236ee20
Reviewed-on: https://go-review.googlesource.com/c/go/+/502615 Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
(cherry picked from commit 962753b015407c69dd334578fd32a80aa7905c24)
Reviewed-on: https://go-review.googlesource.com/c/go/+/502697
Bryan C. Mills [Thu, 8 Jun 2023 21:55:55 +0000 (17:55 -0400)]
[release-branch.go1.20] cmd/go: omit checksums for go.mod files needed for go version lines more often in pre-1.21 modules
This updates the logic from CL 489075 to avoid trying to save extra
sums if they aren't already expected to be present
and cfg.BuildMod != "mod" (as in the case of "go list -m -u all" with
a go.mod file that specifies go < 1.21).
Change-Id: I3f5d14aee7d7195030e8872e42b1d97aa11d3582
Reviewed-on: https://go-review.googlesource.com/c/go/+/501298
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Directory or file paths containing newlines may cause tools (such as
cmd/cgo) that emit "//line" or "#line" -directives to write part of
the path into non-comment lines in generated source code. If those
lines contain valid Go code, it may be injected into the resulting
binary.
(Note that Go import paths and file paths within module zip files
already could not contain newlines.)
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Roland Shoemaker [Fri, 5 May 2023 20:10:34 +0000 (13:10 -0700)]
[release-branch.go1.20] cmd/go: enforce flags with non-optional arguments
Enforce that linker flags which expect arguments get them, otherwise it
may be possible to smuggle unexpected flags through as the linker can
consume what looks like a flag as an argument to a preceding flag (i.e.
"-Wl,-O -Wl,-R,-bad-flag" is interpreted as "-O=-R -bad-flag"). Also be
somewhat more restrictive in the general format of some flags.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Updates #60305
Fixes #60512
Fixes CVE-2023-29404
Change-Id: I5989f68d21a8851d8edd47f08550850524ee9180
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1876275 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 896779503cf754cbdac24b61d4cc953b50fe2dde)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902226
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904346 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501221
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Ian Lance Taylor [Thu, 4 May 2023 21:06:39 +0000 (14:06 -0700)]
[release-branch.go1.20] cmd/go,cmd/cgo: in _cgo_flags use one line per flag
The flags that we recorded in _cgo_flags did not use any quoting,
so a flag containing embedded spaces was mishandled.
Change the _cgo_flags format to put each flag on a separate line.
That is a simple format that does not require any quoting.
As far as I can tell only cmd/go uses _cgo_flags, and it is only
used for gccgo. If this patch doesn't cause any trouble, then
in the next release we can change to only using _cgo_flags for gccgo.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
Updates #60306
Fixes #60514
Fixes CVE-2023-29405
Change-Id: I36b6e188a44c80d7b9573efa577c386770bd2ba3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1875094 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
(cherry picked from commit bcdfcadd5612212089d958bc352a6f6c90742dcc)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902228
Run-TryBot: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904345 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501220
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
On Unix platforms, the runtime previously did nothing special when a
program was run with either the SUID or SGID bits set. This can be
dangerous in certain cases, such as when dumping memory state, or
assuming the status of standard i/o file descriptors.
Taking cues from glibc, this change implements a set of protections when
a binary is run with SUID or SGID bits set (or is SUID/SGID-like). On
Linux, whether to enable these protections is determined by whether the
AT_SECURE flag is passed in the auxiliary vector. On platforms which
have the issetugid syscall (the BSDs, darwin, and Solaris/Illumos), that
is used. On the remaining platforms (currently only AIX) we check
!(getuid() == geteuid() && getgid == getegid()).
Currently when we determine a binary is "tainted" (using the glibc
terminology), we implement two specific protections:
1. we check if the file descriptors 0, 1, and 2 are open, and if they
are not, we open them, pointing at /dev/null (or fail).
2. we force GOTRACKBACK=none, and generally prevent dumping of
trackbacks and registers when a program panics/aborts.
In the future we may add additional protections.
This change requires implementing issetugid on the platforms which
support it, and implementing getuid, geteuid, getgid, and getegid on
AIX.
Thanks to Vincent Dehors from Synacktiv for reporting this issue.
Updates #60272
Fixes #60518
Fixes CVE-2023-29403
Change-Id: Icb620f3f8755791d51b02b5c07fb24f40e19cb80
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1878434 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Russ Cox <rsc@google.com>
(cherry picked from commit 87065663ea6d89cd54f65a515d8f2ed0ef285c19)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902232
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904344 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501227
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
[release-branch.go1.20] cmd/go: save checksums for go.mod files needed for go version lines
When we load a package from a module, we need the go version line from
that module's go.mod file to know what language semantics to use for
the package. We need to save a checksum for the go.mod file even if
the module's requirements are pruned out of the module graph.
Previously, we were missing checksums for test dependencies of
packages in 'all' and packages passed to 'go get -t'.
This change preserves the existing bug for 'go mod tidy',
but fixes it for 'go get -t' and flags the missing checksum
with a clearer error in other cases.
Fixes #60001.
Updates #56222.
Change-Id: Icd6acce348907621ae0b02dbeac04fb180353dcf
(cherry picked from CL 489075 and CL 492741)
Reviewed-on: https://go-review.googlesource.com/c/go/+/493015
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Ian Lance Taylor [Fri, 19 May 2023 22:09:58 +0000 (15:09 -0700)]
[release-branch.go1.20] net, os: net.Conn.File.Fd should return a blocking descriptor
Historically net.Conn.File.Fd has returned a descriptor in blocking mode.
That was broken by CL 495079, which changed the behavior for os.OpenFile
and os.NewFile without intending to affect net.Conn.File.Fd.
Use a hidden os entry point to preserve the historical behavior,
to ensure backward compatibility.
For #58408
For #60211
For #60217
Change-Id: I8d14b9296070ddd52bb8940cb88c6a8b2dc28c27
Reviewed-on: https://go-review.googlesource.com/c/go/+/496080
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
(cherry picked from commit b950cc8f11dc31cc9f6cfbed883818a7aa3abe94)
Reviewed-on: https://go-review.googlesource.com/c/go/+/496715
Ian Lance Taylor [Fri, 19 May 2023 04:13:03 +0000 (21:13 -0700)]
[release-branch.go1.20] runtime: change fcntl to return two values
Separate the result and the errno value, rather than assuming
that the result can never be negative.
Change-Id: Ib01a70a3d46285aa77e95371cdde74e1504e7c12
Reviewed-on: https://go-review.googlesource.com/c/go/+/496416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/497116
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Clean up and consolidate on a single consistent definition of fcntl,
which takes three int32 arguments and returns either a positive result
or a negative errno value.
Change-Id: Id9505492712db4b0aab469c6bd15e4fce3c9ff6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495075
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/497115
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
When combining a byteswap and a load, the resulting combined op
must go in the load's block, not the byteswap's block, as the load
has a memory argument that might only be valid in its original block.
Fixes #59975
Change-Id: Icd84863ef3a9ca1fc22f2bb794a003f2808c746f
Reviewed-on: https://go-review.googlesource.com/c/go/+/492616
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org> Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit 10141676d15665ed8a0255cfc5585ad50444f4ee)
Reviewed-on: https://go-review.googlesource.com/c/go/+/492696
Roland Shoemaker [Thu, 13 Apr 2023 21:01:50 +0000 (14:01 -0700)]
[release-branch.go1.20] html/template: emit filterFailsafe for empty unquoted attr value
An unquoted action used as an attribute value can result in unsafe
behavior if it is empty, as HTML normalization will result in unexpected
attributes, and may allow attribute injection. If executing a template
results in a empty unquoted attribute value, emit filterFailsafe
instead.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
For #59722
Fixes #59816
Fixes CVE-2023-29400
Change-Id: Ia38d1b536ae2b4af5323a6c6d861e3c057c2570a
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1826631 Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1851494
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491358 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Roland Shoemaker [Thu, 13 Apr 2023 22:40:44 +0000 (15:40 -0700)]
[release-branch.go1.20] html/template: disallow angle brackets in CSS values
Angle brackets should not appear in CSS contexts, as they may affect
token boundaries (such as closing a <style> tag, resulting in
injection). Instead emit filterFailsafe, matching the behavior for other
dangerous characters.
Thanks to Juho Nurminen of Mattermost for reporting this issue.
For #59720
Fixes #59812
Fixes CVE-2023-24539
Change-Id: Iccc659c9a18415992b0c05c178792228e3a7bae4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1826636 Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1851492
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491336
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
When server and client have mismatch in curve preference, the server will
send HelloRetryRequest during TLSv1.3 PSK resumption. There was a bug
introduced by Go1.19.6 or later and Go1.20.1 or later, that makes the client
calculate the PSK binder hash incorrectly. Server will reject the TLS
handshake by sending alert: invalid PSK binder.
For #59424.
Fixes #59540.
Change-Id: I2ca8948474275740a36d991c057b62a13392dbb9
GitHub-Last-Rev: 1aad9bcf27f563449c1a7ed6d0dd1d247cc65713
GitHub-Pull-Request: golang/go#59425
Reviewed-on: https://go-review.googlesource.com/c/go/+/481955 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
(cherry picked from commit 2c70690451f1484607a9172a4c24f78ae832dcb0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/488055
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Matthew Dempsky [Wed, 12 Apr 2023 19:41:45 +0000 (12:41 -0700)]
[release-branch.go1.20] cmd/compile: fix reproducible build of aliased generic types
Due to a missing "&& !alias" check, the unified linker was treating
type aliases the same as defined types for the purpose of exporting
method bodies. The methods will get exported anyway alongside the
aliased type, so this mistake is normally harmless.
However, if multiple type aliases instantiated the same generic type
but with different type arguments, this could result in the
same (generic) method body being exported multiple times under
different symbol names. Further, because bodies aren't expected to be
exported multiple times, we were sorting them simply based on index.
And consequently, the sort wasn't total and is sensitive to the map
iteration order used while ranging over linker.bodies.
The fix is simply to add the missing "&& !alias" check, so that we
don't end up with duplicate bodies in the first place.
Thanks rsc@ for providing a minimal repro case.
Fixes #59585.
Change-Id: Iaa55968cc7110b601e2f0f9b620901c2d55f7014
Reviewed-on: https://go-review.googlesource.com/c/go/+/484155 Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit f58c6cccc44752146aabcd50a30865e34817a4b4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/484160 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
CL 440035 added rewrite rules to simplify "costly" LEA
instructions, but the types in the rewrites were wrong and
the code would go bad if the wrong-typed register was spilled.
CL 482536 attempted to fix this by correcting the type in the
rewrite, but that "fix" broke something on windows-amd64-race.
Instead / for-now, remove the offending rewrite rules.
Updates #21735.
Updates #59432.
Fixes #59468.
Change-Id: I0497c42db414f2055e1378e0a53e2bceee9cd5d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/482820
Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 6a97a60b4b80dfa9db91200c054d734e18dda115)
Reviewed-on: https://go-review.googlesource.com/c/go/+/482164
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Keith Randall [Sat, 1 Apr 2023 17:33:26 +0000 (10:33 -0700)]
[release-branch.go1.20] cmd/compile: use correct type for byteswaps on multi-byte stores
Use the type of the store for the byteswap, not the type of the
store's value argument.
Normally when we're storing a 16-bit value, the value being stored is
also typed as 16 bits. But sometimes it is typed as something smaller,
usually because it is the result of an upcast from a smaller value,
and that upcast needs no instructions.
If the type of the store's arg is thinner than the type being stored,
and the byteswap'd value uses that thinner type, and the byteswap'd
value needs to be spilled & restored, that spill/restore happens using
the thinner type, which causes us to lose some of the top bits of the
value.
Fixes #59374
Change-Id: If6ce1e8a76f18bf8e9d79871b6caa438bc3cce4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/481395 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit b3bc8620f89153fddc1a30ee17c1d93654ed4314)
Reviewed-on: https://go-review.googlesource.com/c/go/+/483176
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Ian Lance Taylor [Wed, 22 Mar 2023 20:17:58 +0000 (13:17 -0700)]
[release-branch.go1.20] syscall: restore original NOFILE rlimit in child process
If we increased the NOFILE rlimit when starting the program,
restore the original rlimit when forking a child process.
In CL 393354 the os package was changed to raise the open file rlimit
at program start. That code is not inherently tied to the os package.
This CL moves it into the syscall package.
This is a backport of CLs 476096 and 476097 from trunk.
For #46279
Fixes #59064
Change-Id: Ib813de896de0a5d28fa2b29afdf414a89fbe7b2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/478659
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Keith Randall [Thu, 30 Mar 2023 16:52:39 +0000 (09:52 -0700)]
[release-branch.go1.20] crypto/subtle: don't cast to *uintptr when word size is 0
Casting to a *uintptr is not ok if there isn't at least 8 bytes of
data backing that pointer (on 64-bit archs).
So although we end up making a slice of 0 length with that pointer,
the cast itself doesn't know that.
Instead, bail early if the result is going to be 0 length.
Fixes #59336
Change-Id: Id3c0e09d341d838835c0382cccfb0f71dc3dc7e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/480575
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
(cherry picked from commit 297cf6dd31bd99fc4ccda320aa3d4faf290ab278)
Reviewed-on: https://go-review.googlesource.com/c/go/+/481238 Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 4 Apr 2023 22:07:09 +0000 (22:07 +0000)]
[release-branch.go1.20] html/template,mime/multipart: document new GODEBUG settings
This change documents the new GODEBUG settings introduced for
html/template and mime/multipart, released with Go 1.19.8 and Go 1.20.3
as part of a security fix.
Updates #59153.
For #59270.
Updates #59234.
For #59272.
Change-Id: I25f4d8245da3301dccccfb44da8ff1a5985392a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/482555
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>