From ec988a9af1c5041ab01c4222fa86d92649e8e75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 29 Apr 2026 15:10:29 +0100 Subject: [PATCH 01/12] [release-branch.go1.26] os: notify testlog from (*File).Chdir os.Chdir notifies the testlog hook with the new working directory, but (*File).Chdir does not, so cmd/go's test cache loses track of the cwd when a test uses (*File).Chdir to restore the previous directory (as testing.T.Chdir does in its cleanup). Subsequent relative paths from later tests in the same package are then resolved against the wrong directory in computeTestInputsID, and the cache fails to invalidate when those files change. Fix it by emitting the same testlog Chdir entry from (*File).Chdir after a successful Fchdir, mirroring os.Chdir. Fixes #79039. Updates #79019. Change-Id: I7099d3a1d2ce3e5f4882a0e93c4d642f23f7eca8 Reviewed-on: https://go-review.googlesource.com/c/go/+/771960 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Dmitri Shuralyov Reviewed-by: Michael Pratt (cherry picked from commit c7e6da5ca59645de42e5d28829ff37452fce16ca) Reviewed-on: https://go-review.googlesource.com/c/go/+/784242 Auto-Submit: Dmitri Shuralyov Reviewed-by: Mark Freeman --- src/os/file_plan9.go | 7 +++++++ src/os/file_posix.go | 7 +++++++ src/os/os_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/src/os/file_plan9.go b/src/os/file_plan9.go index e563d123efa2c8..c708f7464de078 100644 --- a/src/os/file_plan9.go +++ b/src/os/file_plan9.go @@ -8,6 +8,7 @@ import ( "internal/bytealg" "internal/poll" "internal/stringslite" + "internal/testlog" "io" "runtime" "sync" @@ -560,6 +561,12 @@ func (f *File) Chdir() error { if e := syscall.Fchdir(f.sysfd); e != nil { return &PathError{Op: "chdir", Path: f.name, Err: e} } + if log := testlog.Logger(); log != nil { + wd, err := Getwd() + if err == nil { + log.Chdir(wd) + } + } return nil } diff --git a/src/os/file_posix.go b/src/os/file_posix.go index 68904f62e1ffca..12fc523f84c430 100644 --- a/src/os/file_posix.go +++ b/src/os/file_posix.go @@ -7,6 +7,7 @@ package os import ( + "internal/testlog" "runtime" "syscall" "time" @@ -208,6 +209,12 @@ func (f *File) Chdir() error { if e := f.pfd.Fchdir(); e != nil { return f.wrapErr("chdir", e) } + if log := testlog.Logger(); log != nil { + wd, err := Getwd() + if err == nil { + log.Chdir(wd) + } + } return nil } diff --git a/src/os/os_test.go b/src/os/os_test.go index d95cf3adcc62a8..62c8544333a813 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -1618,6 +1618,53 @@ func TestFileChdir(t *testing.T) { } } +// TestFileChdirTestlog verifies that (*File).Chdir notifies the testlog, +// just like os.Chdir does. cmd/go's test cache relies on every working +// directory change being recorded so that relative paths logged by later +// Open/Stat calls can be resolved against the correct directory. +func TestFileChdirTestlog(t *testing.T) { + if Getenv("GO_TEST_FILE_CHDIR_TESTLOG") == "1" { + fd, err := Open(".") + if err != nil { + t.Fatalf("Open .: %s", err) + } + defer fd.Close() + if err := Chdir(TempDir()); err != nil { + t.Fatalf("Chdir: %s", err) + } + if err := fd.Chdir(); err != nil { + t.Fatalf("fd.Chdir: %s", err) + } + return + } + + testenv.MustHaveExec(t) + exe := testenv.Executable(t) + logfile := filepath.Join(t.TempDir(), "testlog.txt") + cmd := testenv.Command(t, exe, + "-test.run=^TestFileChdirTestlog$", + "-test.testlogfile="+logfile) + cmd = testenv.CleanCmdEnv(cmd) + cmd.Env = append(cmd.Env, "GO_TEST_FILE_CHDIR_TESTLOG=1") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("helper failed: %v\n%s", err, out) + } + + data, err := ReadFile(logfile) + if err != nil { + t.Fatal(err) + } + chdirs := 0 + for _, line := range strings.Split(string(data), "\n") { + if strings.HasPrefix(line, "chdir ") { + chdirs++ + } + } + if chdirs < 2 { + t.Fatalf("got %d chdir testlog entries, want at least 2; testlog:\n%s", chdirs, data) + } +} + func TestChdirAndGetwd(t *testing.T) { t.Chdir(t.TempDir()) // Ensure wd is restored after the test. From fe91cea4f2536299b29341530f30db7fd190b8f1 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Fri, 3 Apr 2026 15:03:36 -0400 Subject: [PATCH 02/12] [release-branch.go1.26] cmd/go: use fsys.ReadDir for IsStandardPackage FIPS140 crypto files will be bound into the virtual filesystem using the fsys package. So IsStandardPackage needs to use fsys.ReadDir to check that the fips140 packages are standard packages rather than os.ReadDir because os.ReadDir doesn't know about the overlay. It would be nice if we could pass in a io/fs.FS to IsStandardPackage but the FS paths are slash paths and don't play well with windows paths. So we pass in ReadDir instead. Maybe in the future we could create an alternative interface to pass the filesystem through but that's a bigger project. For #73649 Fixes #79721 Change-Id: I576f03cfc52a63cec0598e058e1354676a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/762581 Reviewed-by: Dmitri Shuralyov Reviewed-by: Michael Matloob Auto-Submit: Michael Matloob LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov (cherry picked from commit be193f3a97dce50bad1cf6100aa662c2f6ba57f7) Reviewed-on: https://go-review.googlesource.com/c/go/+/784861 Auto-Submit: Dmitri Shuralyov LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Mark Freeman --- src/cmd/go/internal/modindex/read.go | 4 ++-- .../mod/example.com_importcrypto_v1.0.0.txt | 19 +++++++++++++++++++ .../script/mod_get_fips140_issue73649.txt | 10 ++++++++++ src/go/build/build.go | 2 +- src/internal/goroot/gc.go | 6 +++--- 5 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 src/cmd/go/testdata/mod/example.com_importcrypto_v1.0.0.txt create mode 100644 src/cmd/go/testdata/script/mod_get_fips140_issue73649.txt diff --git a/src/cmd/go/internal/modindex/read.go b/src/cmd/go/internal/modindex/read.go index 399e89eca3cf47..a31e09d233209c 100644 --- a/src/cmd/go/internal/modindex/read.go +++ b/src/cmd/go/internal/modindex/read.go @@ -680,7 +680,7 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b // and otherwise falling back to internal/goroot.IsStandardPackage func IsStandardPackage(goroot_, compiler, path string) bool { if !enabled || compiler != "gc" { - return goroot.IsStandardPackage(goroot_, compiler, path) + return goroot.IsStandardPackage(fsys.ReadDir, goroot_, compiler, path) } reldir := filepath.FromSlash(path) // relative dir path in module index for package @@ -695,7 +695,7 @@ func IsStandardPackage(goroot_, compiler, path string) bool { } else if errors.Is(err, ErrNotIndexed) { // Fall back because package isn't indexable. (Probably because // a file was modified recently) - return goroot.IsStandardPackage(goroot_, compiler, path) + return goroot.IsStandardPackage(fsys.ReadDir, goroot_, compiler, path) } return false } diff --git a/src/cmd/go/testdata/mod/example.com_importcrypto_v1.0.0.txt b/src/cmd/go/testdata/mod/example.com_importcrypto_v1.0.0.txt new file mode 100644 index 00000000000000..0ca0f35bf2ba83 --- /dev/null +++ b/src/cmd/go/testdata/mod/example.com_importcrypto_v1.0.0.txt @@ -0,0 +1,19 @@ +example.com/importcrypto contains a package that imports crypto/sha256 +It's used by the script test case mod_get_fips140_issue73649 + +-- .info -- +{"Version":"v1.0.0"} +-- .mod -- +module example.com/importcrypto + +go 1.27 +-- go.mod -- +module example.com/importcrypto + +go 1.27 +-- importcypto.go -- +package p + +import _ "crypto/sha256" + +func main() {} diff --git a/src/cmd/go/testdata/script/mod_get_fips140_issue73649.txt b/src/cmd/go/testdata/script/mod_get_fips140_issue73649.txt new file mode 100644 index 00000000000000..f6904e2d88d6da --- /dev/null +++ b/src/cmd/go/testdata/script/mod_get_fips140_issue73649.txt @@ -0,0 +1,10 @@ +[GOEXPERIMENT:boringcrypto] skip + +env GOFIPS140=v1.0.0 +env GOCACHE=$WORK/cache +go get example.com/importcrypto + +-- go.mod -- +module m + +go 1.24 diff --git a/src/go/build/build.go b/src/go/build/build.go index 68fb8dbbd7ab69..69e7191f0a5e3b 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -746,7 +746,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa } tried.goroot = dir } - if ctxt.Compiler == "gccgo" && goroot.IsStandardPackage(ctxt.GOROOT, ctxt.Compiler, path) { + if ctxt.Compiler == "gccgo" && goroot.IsStandardPackage(os.ReadDir, ctxt.GOROOT, ctxt.Compiler, path) { // TODO(bcmills): Setting p.Dir here is misleading, because gccgo // doesn't actually load its standard-library packages from this // directory. See if we can leave it unset. diff --git a/src/internal/goroot/gc.go b/src/internal/goroot/gc.go index 534ad57e709fa6..d2581b04252467 100644 --- a/src/internal/goroot/gc.go +++ b/src/internal/goroot/gc.go @@ -15,12 +15,12 @@ import ( ) // IsStandardPackage reports whether path is a standard package, -// given goroot and compiler. -func IsStandardPackage(goroot, compiler, path string) bool { +// given goroot and compiler. readDir accepts OS filesystem paths. +func IsStandardPackage(readDir func(string) ([]os.DirEntry, error), goroot, compiler, path string) bool { switch compiler { case "gc": dir := filepath.Join(goroot, "src", path) - dirents, err := os.ReadDir(dir) + dirents, err := readDir(dir) if err != nil { return false } From 6efcff66029b119a2675e8c1f920147c766a1dbd Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 19 May 2026 09:28:23 -0700 Subject: [PATCH 03/12] [release-branch.go1.26] os/signal: make NotifyContext Cause match context.Canceled For #77639 Fixes #79499 Change-Id: I90e5d6f68c02749ac6e407f1c3565312efafb033 Reviewed-on: https://go-review.googlesource.com/c/go/+/779860 Reviewed-by: David Chase LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Damien Neil Reviewed-by: Florian Lehner Auto-Submit: Ian Lance Taylor (cherry picked from commit 8494d25c4cc090a84d82d88a9ddbed21e0c32c8c) Reviewed-on: https://go-review.googlesource.com/c/go/+/784100 Reviewed-by: Junyang Shao Reviewed-by: Dmitri Shuralyov Auto-Submit: Dmitri Shuralyov --- src/os/signal/signal.go | 7 +++++++ src/os/signal/signal_test.go | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/os/signal/signal.go b/src/os/signal/signal.go index 70a91055e235bb..100cdd30046e9d 100644 --- a/src/os/signal/signal.go +++ b/src/os/signal/signal.go @@ -342,3 +342,10 @@ type signalError string func (s signalError) Error() string { return string(s) } + +func (s signalError) Is(target error) bool { + if target == context.Canceled { + return true + } + return false +} diff --git a/src/os/signal/signal_test.go b/src/os/signal/signal_test.go index 8a3ba0e847546b..693e40c90aee50 100644 --- a/src/os/signal/signal_test.go +++ b/src/os/signal/signal_test.go @@ -924,3 +924,25 @@ func TestSignalTrace(t *testing.T) { close(quit) <-done } + +// #77639. +func TestNotifyContextCause(t *testing.T) { + ctx, stop := NotifyContext(t.Context(), syscall.SIGINT) + defer stop() + syscall.Kill(syscall.Getpid(), syscall.SIGINT) + <-ctx.Done() + + err := ctx.Err() + if err == nil { + t.Error("ctx.Err returned nil") + } else if !errors.Is(err, context.Canceled) { + t.Errorf("error %v is not context.Canceled", err) + } + + err = context.Cause(ctx) + if err == nil { + t.Error("context.Cause returned nil") + } else if !errors.Is(err, context.Canceled) { + t.Errorf("cause %v is not context.Canceled", err) + } +} From dc80258a9793b18d83c82fcb3a2fde4c0aa11b4e Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Mon, 8 Jun 2026 18:27:23 -0700 Subject: [PATCH 04/12] [release-branch.go1.26] internal/runtime/gc: require AVX512DQ for greentea The KMOVB instruction (in scan_amd64.s) requires that flag. Fixes #79920 Change-Id: Id03285932589e349c7a2cc214dc7ea5b448c0a39 Reviewed-on: https://go-review.googlesource.com/c/go/+/788600 Reviewed-by: Keith Randall LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Michael Pratt Auto-Submit: Keith Randall Reviewed-on: https://go-review.googlesource.com/c/go/+/788860 Auto-Submit: Dmitri Shuralyov --- src/internal/runtime/gc/scan/scan_amd64.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/internal/runtime/gc/scan/scan_amd64.go b/src/internal/runtime/gc/scan/scan_amd64.go index 0151804fa537fc..c03021c7d40941 100644 --- a/src/internal/runtime/gc/scan/scan_amd64.go +++ b/src/internal/runtime/gc/scan/scan_amd64.go @@ -38,4 +38,5 @@ var avx512ScanPackedReqsMet = cpu.X86.HasAVX512VL && cpu.X86.HasAVX512BW && cpu.X86.HasGFNI && cpu.X86.HasAVX512BITALG && + cpu.X86.HasAVX512DQ && // for kmovb, see #79871 cpu.X86.HasAVX512VBMI From d7c9c818f9cbe79853b07115f31058a94c79d87d Mon Sep 17 00:00:00 2001 From: "khr@golang.org" Date: Sun, 14 Jun 2026 08:30:17 -0400 Subject: [PATCH 05/12] [release-branch.go1.26] runtime: in moveSliceNoCap, round size down to a whole multiple of element size The allocator doesn't accept total sizes that are not a multiple of the element size. Fixes #80007 Change-Id: I177d67c1ecc15ec9d09b160bbcc2bfb7a3a1891f Reviewed-on: https://go-review.googlesource.com/c/go/+/790600 Reviewed-by: Cuong Manh Le Reviewed-by: Keith Randall LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Dmitri Shuralyov (cherry picked from commit 1a15699a43a4a5f1fa1b7ec2282268c6b14c689d) Reviewed-on: https://go-review.googlesource.com/c/go/+/791260 Auto-Submit: Dmitri Shuralyov --- src/runtime/slice.go | 5 ++- test/fixedbugs/issue80004.go | 82 ++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 test/fixedbugs/issue80004.go diff --git a/src/runtime/slice.go b/src/runtime/slice.go index 2a442977542888..9f5561feaf04a8 100644 --- a/src/runtime/slice.go +++ b/src/runtime/slice.go @@ -480,10 +480,11 @@ func moveSliceNoCap(et *_type, old unsafe.Pointer, len int) (unsafe.Pointer, int } lenmem := uintptr(len) * et.Size_ capmem := roundupsize(lenmem, false) - new := mallocgc(capmem, et, true) + cap := capmem / et.Size_ + new := mallocgc(cap*et.Size_, et, true) bulkBarrierPreWriteSrcOnly(uintptr(new), uintptr(old), lenmem, et) memmove(new, old, lenmem) - return new, len, int(capmem / et.Size_) + return new, len, int(cap) } // moveSliceNoCapNoScan is a combination of moveSliceNoScan and moveSliceNoCap. diff --git a/test/fixedbugs/issue80004.go b/test/fixedbugs/issue80004.go new file mode 100644 index 00000000000000..59ec7f22c6edee --- /dev/null +++ b/test/fixedbugs/issue80004.go @@ -0,0 +1,82 @@ +// run + +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "runtime" + "unsafe" +) + +// 1 of these is 28 bytes. +// When allocating 1 of them in a 32-byte size class, +// we accidentally write 2 pointer bitmasks, which +// marks the first 6 fields of the following object +// as pointers. +type E struct { + a [7]*byte +} + +// 32 bytes, and has pointers. +// First field is not a pointer, but will contain +// a badPtr value. +type Victim struct { + a uintptr + b [6]int32 + c *byte +} + +//go:noinline +func f(n int) []E { + var r []E + for range n { + r = append(r, E{}) + } + return r +} + +//go:noinline +func newVictim() *Victim { + return &Victim{a: badPtr} +} + +var badPtr uintptr + +func init() { + x := make([]byte, 1<<18-8) + sink = &x[0] + badPtr = uintptr(unsafe.Pointer(&x[len(x)-1])) + 5 +} + +func main() { + fs := make([]*Victim, 1000) + + // Allocate a bunch of Victims. + for i := range fs { + fs[i] = newVictim() + } + // Deallocate every other one. + for i := range fs { + if i%2 == 1 { + fs[i] = nil + } + } + runtime.GC() + + // Allocate Es in the deallocated slots. + // Those allocations will incorrectly set the + // pointer bit for the first field of all the + // Victims we allocated. + for range len(fs) / 2 { + _ = f(1) + } + runtime.GC() + + // Keep fs alive. + sink = &fs[0] +} + +var sink any From 2055b1a15decdb874718dad06bfe573ae74e10dd Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Wed, 3 Jun 2026 22:25:26 +0000 Subject: [PATCH 06/12] [release-branch.go1.26] syscall: add //go:norace to rawSyscall on darwin On darwin, rawSyscall, rawSyscall6, and rawSyscall9 are Go functions (not assembly as on Linux) that get race-detector instrumentation. When forkAndExecInChild calls rawSyscall in the forked child process, the TSan ThreadState pointer is invalid, causing SIGSEGV in TraceSwitchPartImpl. Add //go:norace to these functions so the race detector does not instrument them. Also update the stale comment in exec_libc2.go that referred to rawSyscall as assembly. For #79804. Fixes #79806. Change-Id: Ia1d5ffea9bc9729ce5392daaf00b45ab1f779d71 GitHub-Last-Rev: 21d574438c6ff11cd4939d463955152315827680 GitHub-Pull-Request: golang/go#79805 Reviewed-on: https://go-review.googlesource.com/c/go/+/786620 Reviewed-by: Cherry Mui LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Quim Muntal Reviewed-by: Dmitri Shuralyov Auto-Submit: Dmitri Shuralyov (cherry picked from commit 0607ffa8e9bd17e112e13238ad309985f3ca3b9b) Reviewed-on: https://go-review.googlesource.com/c/go/+/791800 Auto-Submit: Dmitri Shuralyov --- src/syscall/exec_libc2.go | 5 +++-- src/syscall/syscall_darwin.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go index 5de09dfe998070..9e5c502159706d 100644 --- a/src/syscall/exec_libc2.go +++ b/src/syscall/exec_libc2.go @@ -48,8 +48,9 @@ func runtime_AfterForkInChild() // they might have been locked at the time of the fork. This means // no rescheduling, no malloc calls, and no new stack segments. // For the same reason compiler does not race instrument it. -// The calls to rawSyscall are okay because they are assembly -// functions that do not grow the stack. +// The calls to rawSyscall are okay because they are nosplit +// functions that do not grow the stack and are not race +// instrumented (go:norace). // //go:norace func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err1 Errno) { diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go index ca76cc2962b098..b65f69f978d81b 100644 --- a/src/syscall/syscall_darwin.go +++ b/src/syscall/syscall_darwin.go @@ -326,6 +326,7 @@ func Getdirentries(fd int, buf []byte, basep *uintptr) (n int, err error) { // errno return e if int32(r) is -1, else it returns 0. // //go:nosplit +//go:norace func errno(r uintptr, e Errno) Errno { if int32(r) == -1 { return e @@ -336,6 +337,7 @@ func errno(r uintptr, e Errno) Errno { // errnoX return e if r is -1, else it returns 0. // //go:nosplit +//go:norace func errnoX(r uintptr, e Errno) Errno { if r == ^uintptr(0) { return e @@ -346,6 +348,7 @@ func errnoX(r uintptr, e Errno) Errno { // errnoPtr return e if r is 0, else it returns 0. // //go:nosplit +//go:norace func errnoPtr(r uintptr, e Errno) Errno { if r == 0 { return e @@ -358,6 +361,22 @@ func errnoPtr(r uintptr, e Errno) Errno { // golang.org/x/sys linknames the following syscalls. // Do not remove or change the type signature. +// N.B. For the Syscall functions below: +// +// //go:uintptrkeepalive because the uintptr argument may be converted pointers +// that need to be kept alive in the caller. +// +// //go:nosplit because stack copying does not account for uintptrkeepalive, so +// the stack must not grow. Stack copying cannot blindly assume that all +// uintptr arguments are pointers, because some values may look like pointers, +// but not really be pointers, and adjusting their value would break the call. +// +// //go:norace, on RawSyscall, to avoid race instrumentation if RawSyscall is +// called after fork, or from a signal handler. +// +// //go:linkname to ensure ABI wrappers are generated for external callers +// (notably x/sys/unix assembly). + //go:linkname syscall //go:nosplit //go:uintptrkeepalive @@ -409,6 +428,7 @@ func syscall9(fn, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, e //go:linkname rawSyscall //go:nosplit +//go:norace //go:uintptrkeepalive func rawSyscall(fn, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno) { r1, r2, err = rawsyscalln(fn, a1, a2, a3) @@ -417,6 +437,7 @@ func rawSyscall(fn, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno) { //go:linkname rawSyscall6 //go:nosplit +//go:norace //go:uintptrkeepalive func rawSyscall6(fn, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno) { r1, r2, err = rawsyscalln(fn, a1, a2, a3, a4, a5, a6) @@ -425,6 +446,7 @@ func rawSyscall6(fn, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno) //go:linkname rawSyscall9 //go:nosplit +//go:norace //go:uintptrkeepalive func rawSyscall9(fn, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err Errno) { r1, r2, err = rawsyscalln(fn, a1, a2, a3, a4, a5, a6, a7, a8, a9) From 814591410b510099546873c10463fc6299fe2e1e Mon Sep 17 00:00:00 2001 From: Jorropo Date: Mon, 4 May 2026 23:38:34 +0200 Subject: [PATCH 07/12] [release-branch.go1.26] cmd/compile,sync/atomic: make Add And & Or SQCST on PPC64 Updates #79186 Fixes #79879 Change-Id: If7e298270ac6252b092371725d6a96aa871bf919 Reviewed-on: https://go-review.googlesource.com/c/go/+/774020 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Dmitri Shuralyov Reviewed-by: Cherry Mui Reviewed-by: Jayanth Krishnamurthy Auto-Submit: Jorropo Reviewed-by: Paul Murphy Reviewed-on: https://go-review.googlesource.com/c/go/+/793901 Auto-Submit: Junyang Shao Reviewed-by: Junyang Shao Reviewed-by: Keith Randall Reviewed-by: Keith Randall --- src/cmd/compile/internal/ppc64/ssa.go | 19 ++++-- src/internal/runtime/atomic/atomic_ppc64x.s | 10 +++ test/fixedbugs/issue79186.go | 67 +++++++++++++++++++++ 3 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 test/fixedbugs/issue79186.go diff --git a/src/cmd/compile/internal/ppc64/ssa.go b/src/cmd/compile/internal/ppc64/ssa.go index f0d228559f3a87..fe251fa5f2fe9c 100644 --- a/src/cmd/compile/internal/ppc64/ssa.go +++ b/src/cmd/compile/internal/ppc64/ssa.go @@ -135,6 +135,7 @@ func ssaGenValue(s *ssagen.State, v *ssa.Value) { // AND/OR Rarg1, Rtmp // STBCCC/STWCCC Rtmp, (Rarg0) // BNE -3(PC) + // LWSYNC ld := ppc64.ALBAR st := ppc64.ASTBCCC if v.Op == ssa.OpPPC64LoweredAtomicAnd32 || v.Op == ssa.OpPPC64LoweredAtomicOr32 { @@ -170,6 +171,10 @@ func ssaGenValue(s *ssagen.State, v *ssa.Value) { p3 := s.Prog(ppc64.ABNE) p3.To.Type = obj.TYPE_BRANCH p3.To.SetTarget(p) + // LWSYNC - Provide acquire ordering to pair with the + // release (pre-LWSYNC) above, making the operation + // sequentially consistent. + s.Prog(ppc64.ALWSYNC) case ssa.OpPPC64LoweredAtomicAdd32, ssa.OpPPC64LoweredAtomicAdd64: @@ -179,6 +184,7 @@ func ssaGenValue(s *ssagen.State, v *ssa.Value) { // STDCCC/STWCCC Rout, (Rarg0) // BNE -3(PC) // MOVW Rout,Rout (if Add32) + // LWSYNC ld := ppc64.ALDAR st := ppc64.ASTDCCC if v.Op == ssa.OpPPC64LoweredAtomicAdd32 { @@ -188,10 +194,10 @@ func ssaGenValue(s *ssagen.State, v *ssa.Value) { r0 := v.Args[0].Reg() r1 := v.Args[1].Reg() out := v.Reg0() - // LWSYNC - Assuming shared data not write-through-required nor - // caching-inhibited. See Appendix B.2.2.2 in the ISA 2.07b. - plwsync := s.Prog(ppc64.ALWSYNC) - plwsync.To.Type = obj.TYPE_NONE + // LWSYNC - Provide acquire ordering to pair with the + // release (pre-LWSYNC) above, making the operation + // sequentially consistent. + s.Prog(ppc64.ALWSYNC) // LDAR or LWAR p := s.Prog(ld) p.From.Type = obj.TYPE_MEM @@ -223,6 +229,11 @@ func ssaGenValue(s *ssagen.State, v *ssa.Value) { p5.From.Type = obj.TYPE_REG p5.From.Reg = out } + // LWSYNC - Provide acquire ordering to pair with the + // release (pre-LWSYNC) above, making the operation + // sequentially consistent. + plwsync2 := s.Prog(ppc64.ALWSYNC) + plwsync2.To.Type = obj.TYPE_NONE case ssa.OpPPC64LoweredAtomicExchange8, ssa.OpPPC64LoweredAtomicExchange32, diff --git a/src/internal/runtime/atomic/atomic_ppc64x.s b/src/internal/runtime/atomic/atomic_ppc64x.s index bff7d1902a0e15..a82a34e1549e79 100644 --- a/src/internal/runtime/atomic/atomic_ppc64x.s +++ b/src/internal/runtime/atomic/atomic_ppc64x.s @@ -220,6 +220,7 @@ TEXT ·Xadd(SB), NOSPLIT, $0-20 ADD R5, R3 STWCCC R3, (R4) BNE -3(PC) + LWSYNC MOVW R3, ret+16(FP) RET @@ -235,6 +236,7 @@ TEXT ·Xadd64(SB), NOSPLIT, $0-24 ADD R5, R3 STDCCC R3, (R4) BNE -3(PC) + LWSYNC MOVD R3, ret+16(FP) RET @@ -343,6 +345,7 @@ again: OR R4, R6 STBCCC R6, (R3) BNE again + LWSYNC RET // void ·And8(byte volatile*, byte); @@ -355,6 +358,7 @@ again: AND R4, R6 STBCCC R6, (R3) BNE again + LWSYNC RET // func Or(addr *uint32, v uint32) @@ -367,6 +371,7 @@ again: OR R4, R6 STWCCC R6, (R3) BNE again + LWSYNC RET // func And(addr *uint32, v uint32) @@ -379,6 +384,7 @@ again: AND R4, R6 STWCCC R6, (R3) BNE again + LWSYNC RET // func Or32(addr *uint32, v uint32) old uint32 @@ -391,6 +397,7 @@ again: OR R4, R6, R7 STWCCC R7, (R3) BNE again + LWSYNC MOVW R6, ret+16(FP) RET @@ -404,6 +411,7 @@ again: AND R4, R6, R7 STWCCC R7, (R3) BNE again + LWSYNC MOVW R6, ret+16(FP) RET @@ -417,6 +425,7 @@ again: OR R4, R6, R7 STDCCC R7, (R3) BNE again + LWSYNC MOVD R6, ret+16(FP) RET @@ -430,6 +439,7 @@ again: AND R4, R6, R7 STDCCC R7, (R3) BNE again + LWSYNC MOVD R6, ret+16(FP) RET diff --git a/test/fixedbugs/issue79186.go b/test/fixedbugs/issue79186.go new file mode 100644 index 00000000000000..21bb6c84bb8d18 --- /dev/null +++ b/test/fixedbugs/issue79186.go @@ -0,0 +1,67 @@ +// run + +// Copyright 2026 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. + +// Issue 79186: on ppc64le (POWER8/9), atomic add operations lacked a +// post-barrier (acquire ordering), allowing loads after an RWMutex.RLock +// to be speculatively reordered before the lock acquisition, causing +// concurrent map read and map write. + +package main + +import ( + "runtime" + "sync" +) + +type M struct { + mu sync.RWMutex + m map[int]int +} + +func NewM() *M { + return &M{m: make(map[int]int)} +} + +func (x *M) Get(k int) (int, bool) { + x.mu.RLock() + v, ok := x.m[k] + x.mu.RUnlock() + return v, ok +} + +func (x *M) Set(k, v int) { + x.mu.Lock() + x.m[k] = v + x.mu.Unlock() +} + +func main() { + runtime.GOMAXPROCS(2) + + x := NewM() + + const goroutines = 256 + const iters = 200000 + + var wg sync.WaitGroup + wg.Add(goroutines) + + for g := 0; g < goroutines; g++ { + go func(id int) { + defer wg.Done() + for i := 0; i < iters; i++ { + k := (id + i) & 15 + if _, ok := x.Get(k); !ok { + x.Set(k, i) + } else if i&7 == 0 { + x.Set(k, i) + } + } + }(g) + } + + wg.Wait() +} From 044c995af789121b9f5cef2206ef844ef6c8263b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 22 May 2026 03:50:05 +0000 Subject: [PATCH 08/12] [release-branch.go1.26] runtime: tolerate vendor suffixes in Linux kernel release strings Synology kernels can have a "_" in their uname version. Make the parsing more tolerant. And also don't throw during init if we fail to parse the kernel version. Instead, fall back to probing. (We can't probe all the time, because seccomp filters on some platforms like Android kill the process if we call a verbotenen system call) This regressed in CL 758902 (forked from CL 751340) on 2026-03-24, which started calling parseRelease unconditionally during osinit on 32-bit Linux. Updates #79612 Fixes #79893 Change-Id: I98f61b94e54c7b9d08029f3aef664bdda9ec7f69 Reviewed-on: https://go-review.googlesource.com/c/go/+/781800 Reviewed-by: Jorropo Reviewed-by: Damien Neil Reviewed-by: Michael Pratt LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-on: https://go-review.googlesource.com/c/go/+/793960 Auto-Submit: Junyang Shao Reviewed-by: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov Reviewed-by: Junyang Shao --- src/runtime/export_linux_test.go | 1 + src/runtime/os_linux.go | 75 ++++++++++++++++++------------- src/runtime/os_linux32.go | 32 ++++++++++++- src/runtime/runtime_linux_test.go | 30 +++++++++++++ 4 files changed, 106 insertions(+), 32 deletions(-) diff --git a/src/runtime/export_linux_test.go b/src/runtime/export_linux_test.go index 52afd28666e9af..f8c4b4fdc37ea2 100644 --- a/src/runtime/export_linux_test.go +++ b/src/runtime/export_linux_test.go @@ -11,6 +11,7 @@ const SigeventMaxSize = _sigev_max_size var NewOSProc0 = newosproc0 var Mincore = mincore +var ParseRelease = parseRelease type Siginfo siginfo type Sigevent sigevent diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go index 493567b5303673..0474e6c90696cb 100644 --- a/src/runtime/os_linux.go +++ b/src/runtime/os_linux.go @@ -943,53 +943,66 @@ type kernelVersion struct { } // getKernelVersion returns major and minor kernel version numbers -// parsed from the uname release field. -func getKernelVersion() kernelVersion { +// parsed from the uname release field. ok reports whether parsing +// succeeded; on failure callers must pick a default rather than +// failing, because this is called during osinit before the runtime +// is fully initialized and a throw here is unrecoverable. +func getKernelVersion() (kv kernelVersion, ok bool) { var buf linux.Utsname if e := linux.Uname(&buf); e != 0 { - throw("uname failed") + return kernelVersion{}, false } - rel := gostringnocopy(&buf.Release[0]) major, minor, _, ok := parseRelease(rel) if !ok { - throw("failed to parse kernel version from uname") + return kernelVersion{}, false } - return kernelVersion{major: major, minor: minor} + return kernelVersion{major: major, minor: minor}, true } -// parseRelease parses a dot-separated version number. It follows the -// semver syntax, but allows the minor and patch versions to be -// elided. +// parseRelease parses a dot-separated version number from the prefix +// of rel. It returns ok=true only if at least the major and minor +// components were successfully parsed; the patch component is +// best-effort. Trailing vendor or build suffixes such as +// "-generic", "+", "_hi3535", or "-rc1" are ignored. func parseRelease(rel string) (major, minor, patch int, ok bool) { - // Strip anything after a dash or plus. - for i := 0; i < len(rel); i++ { - if rel[i] == '-' || rel[i] == '+' { - rel = rel[:i] - break + // next consumes a run of decimal digits from the front of rel, + // returning the parsed value. If the digits are followed by a + // '.', it is consumed and more is set so the caller knows to + // parse another component; otherwise scanning terminates and + // the rest of rel is discarded. + next := func() (n int, more, ok bool) { + i := 0 + for i < len(rel) && rel[i] >= '0' && rel[i] <= '9' { + i++ } - } - - next := func() (int, bool) { - for i := 0; i < len(rel); i++ { - if rel[i] == '.' { - ver, err := strconv.Atoi(rel[:i]) - rel = rel[i+1:] - return ver, err == nil - } + if i == 0 { + return 0, false, false + } + n, err := strconv.Atoi(rel[:i]) + if err != nil { + return 0, false, false + } + if i < len(rel) && rel[i] == '.' { + rel = rel[i+1:] + return n, true, true } - ver, err := strconv.Atoi(rel) rel = "" - return ver, err == nil + return n, false, true } - if major, ok = next(); !ok || rel == "" { - return + + var more bool + if major, more, ok = next(); !ok || !more { + return 0, 0, 0, false } - if minor, ok = next(); !ok || rel == "" { - return + if minor, more, ok = next(); !ok { + return 0, 0, 0, false + } + if !more { + return major, minor, 0, true } - patch, ok = next() - return + patch, _, _ = next() + return major, minor, patch, true } // GE checks if the running kernel version diff --git a/src/runtime/os_linux32.go b/src/runtime/os_linux32.go index 1ee1cdcaf90051..ff437ce6533282 100644 --- a/src/runtime/os_linux32.go +++ b/src/runtime/os_linux32.go @@ -10,8 +10,38 @@ import ( "unsafe" ) +// configure64bitsTimeOn32BitsArchitectures decides whether to use the +// 64-bit time variants of futex and timer_settime on 32-bit Linux. +// +// The choice is normally made by parsing the kernel release string +// from uname, because probing with -ENOSYS misbehaves on some +// kernels: Android 8.0-10 (API 26-29) has a seccomp filter that kills +// the process on unknown syscalls instead of returning -ENOSYS, and +// some older Synology kernels reuse the new syscall numbers for +// unrelated vendor syscalls, which silently runs the wrong thing. +// +// If the kernel release string can't be parsed, fall back to probing +// futex_time64 with a no-op FUTEX_WAKE: the kernels that motivated +// the version check above all report parseable uname strings, so +// reaching the probing fallback generally means we are on neither, +// and probing is the safest available signal. func configure64bitsTimeOn32BitsArchitectures() { - use64bitsTimeOn32bits = getKernelVersion().GE(5, 1) + if kv, ok := getKernelVersion(); ok { + use64bitsTimeOn32bits = kv.GE(5, 1) + return + } + use64bitsTimeOn32bits = probeFutexTime64() +} + +// probeFutexTime64 reports whether the futex_time64 syscall is +// available on the running kernel. It issues a FUTEX_WAKE_PRIVATE +// with a wake count of 0, which is a no-op if the syscall exists and +// returns -ENOSYS otherwise. timer_settime64 was added to the kernel +// in the same release (Linux 5.1), so a single probe covers both. +func probeFutexTime64() bool { + var word uint32 + ret := futex_time64(unsafe.Pointer(&word), _FUTEX_WAKE_PRIVATE, 0, nil, nil, 0) + return ret != -_ENOSYS } //go:noescape diff --git a/src/runtime/runtime_linux_test.go b/src/runtime/runtime_linux_test.go index ab2452c9e72910..728d65a9d588b4 100644 --- a/src/runtime/runtime_linux_test.go +++ b/src/runtime/runtime_linux_test.go @@ -53,6 +53,36 @@ func TestMincoreErrorSign(t *testing.T) { } } +func TestParseRelease(t *testing.T) { + tests := []struct { + in string + major, minor, patch int + ok bool + }{ + {"6.1.0", 6, 1, 0, true}, + {"5.15.0-91-generic", 5, 15, 0, true}, + {"4.19.0+", 4, 19, 0, true}, + {"6.6.0-rc1", 6, 6, 0, true}, + // Synology embedded Linux appends a platform identifier + // after an underscore. + {"3.4.35_hi3535", 3, 4, 35, true}, + {"2.6.32_synology", 2, 6, 32, true}, + {"3.10", 3, 10, 0, true}, + // A single component is not enough; major+minor required. + {"3", 0, 0, 0, false}, + {"3-rc1", 0, 0, 0, false}, + {"", 0, 0, 0, false}, + {"bogus", 0, 0, 0, false}, + } + for _, tt := range tests { + major, minor, patch, ok := ParseRelease(tt.in) + if major != tt.major || minor != tt.minor || patch != tt.patch || ok != tt.ok { + t.Errorf("ParseRelease(%q) = (%d, %d, %d, %v); want (%d, %d, %d, %v)", + tt.in, major, minor, patch, ok, tt.major, tt.minor, tt.patch, tt.ok) + } + } +} + func TestKernelStructSize(t *testing.T) { // Check that the Go definitions of structures exchanged with the kernel are // the same size as what the kernel defines. From 7397dd7495724542484dbd0d9ad09c35f7ccd003 Mon Sep 17 00:00:00 2001 From: "Nicholas S. Husin" Date: Tue, 30 Jun 2026 16:18:33 -0400 Subject: [PATCH 09/12] [release-branch.go1.26] net: fix TestLookupCNAME The CNAME record for www.iana.org seems to have been changed, causing test failures. Change the test to just use www.golang.org, so we have more control and awareness of such changes in the future. For #80212 Fixes #80217 Change-Id: I7b11bb8f90fe366db019dab1f8d9a1cf6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/795841 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Dmitri Shuralyov Reviewed-by: Nicholas Husin Reviewed-by: Junyang Shao Auto-Submit: Junyang Shao --- src/net/lookup_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index 42211ed099ed1e..afa7e4c14aaf1f 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -335,8 +335,8 @@ func TestLookupIPv6LinkLocalAddrWithZone(t *testing.T) { var lookupCNAMETests = []struct { name, cname string }{ - {"www.iana.org", "icann.org."}, - {"www.iana.org.", "icann.org."}, + {"www.golang.org", "golang.org."}, + {"www.golang.org.", "golang.org."}, {"www.google.com", "google.com."}, {"google.com", "google.com."}, {"cname-to-txt.go4.org", "test-txt-record.go4.org."}, From ca8ca590ccfda1e1c3186faf975afdb02cb6d2f0 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 8 May 2026 09:22:41 -0700 Subject: [PATCH 10/12] [release-branch.go1.26] crypto/tls: omit PSK in ECH outer client hello When using ECH, do not include the PSK extension in the outer hello. Including the PSK extension allows for a degradation in privacy, as an on-path attacker can harvest outer client hellos, and then construct new hellos using the PSK extension and arbitrary guessed SNI values, replaying them to the target server. If the server rejects the PSK, the handshake will continue, but if the PSK is accepted, the binder check will fail. Thanks to Coia Prant (github.com/rbqvq) for reporting this issue. Fixes CVE-2026-42505 Updates #79282 Fixes #80175 Change-Id: Ib3a3c948106a57c1b07b9e61a58cbf757848be18 Reviewed-on: https://go-review.googlesource.com/c/go/+/775960 Auto-Submit: Roland Shoemaker TryBot-Bypass: Roland Shoemaker Reviewed-by: Daniel McCarney Reviewed-by: Carlos Amedee (cherry picked from commit 137b8065ab5b485bbde0ed430dd89841c0602bb2) Reviewed-on: https://go-review.googlesource.com/c/go/+/794921 TryBot-Bypass: Junyang Shao Reviewed-by: Junyang Shao Commit-Queue: Roland Shoemaker Auto-Submit: Junyang Shao Commit-Queue: Junyang Shao --- src/crypto/tls/handshake_messages.go | 4 +- src/crypto/tls/handshake_messages_test.go | 99 ++++++++++++++++++++++- src/crypto/tls/tls_test.go | 1 + 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/crypto/tls/handshake_messages.go b/src/crypto/tls/handshake_messages.go index aa0b7db75dd493..511e073df57d72 100644 --- a/src/crypto/tls/handshake_messages.go +++ b/src/crypto/tls/handshake_messages.go @@ -5,6 +5,7 @@ package tls import ( + "bytes" "errors" "fmt" "slices" @@ -317,7 +318,8 @@ func (m *clientHelloMsg) marshalMsg(echInner bool) ([]byte, error) { }) }) } - if len(m.pskIdentities) > 0 { // pre_shared_key must be the last extension + // pre_shared_key must be the last extension + if len(m.pskIdentities) > 0 && (echInner || len(m.encryptedClientHello) == 0 || bytes.Equal(m.encryptedClientHello, []byte{byte(innerECHExt)})) { // RFC 8446, Section 4.2.11 exts.AddUint16(extensionPreSharedKey) exts.AddUint16LengthPrefixed(func(exts *cryptobyte.Builder) { diff --git a/src/crypto/tls/handshake_messages_test.go b/src/crypto/tls/handshake_messages_test.go index fa81a72b0de018..80a7e76f816ffd 100644 --- a/src/crypto/tls/handshake_messages_test.go +++ b/src/crypto/tls/handshake_messages_test.go @@ -215,7 +215,16 @@ func (*clientHelloMsg) Generate(rand *rand.Rand, size int) reflect.Value { case 2: m.pskModes = []uint8{pskModeDHE, pskModePlain} } - for i := 0; i < rand.Intn(5); i++ { + // clientHelloMsg.marshal uses echInner == false. If m.encryptedClientHello > 0 and + // does not equal []byte{innerECHExt}, then the psk extension will be omitted, + // so either only emit empty encryptedClientHello and psk, encryptedClientHello with + // []byte{innerECHExt} and psk, or random encryptedClientHello and no psk. + if rand.Intn(10) > 5 { + m.encryptedClientHello = randomBytes(rand.Intn(50)+1, rand) + } else { + if rand.Intn(10) > 5 { + m.encryptedClientHello = []byte{byte(innerECHExt)} + } var psk pskIdentity psk.obfuscatedTicketAge = uint32(rand.Intn(500000)) psk.label = randomBytes(rand.Intn(500)+1, rand) @@ -228,9 +237,6 @@ func (*clientHelloMsg) Generate(rand *rand.Rand, size int) reflect.Value { if rand.Intn(10) > 5 { m.earlyData = true } - if rand.Intn(10) > 5 { - m.encryptedClientHello = randomBytes(rand.Intn(50)+1, rand) - } return reflect.ValueOf(m) } @@ -587,3 +593,88 @@ func TestRejectDuplicateExtensions(t *testing.T) { t.Fatal("Unmarshaled ServerHello with duplicate extensions") } } + +func TestECHRemoveOuterPSK(t *testing.T) { + r := rand.New(rand.NewSource(0)) + + for _, tc := range []struct { + name string + echInner bool + echExt []byte + expectRemoved bool + }{ + { + name: "echInner true", + echInner: true, + expectRemoved: false, + }, + { + name: "echInner true, no ech ext", + echInner: true, + expectRemoved: false, + }, + { + name: "echInner true, ech ext present", + echInner: true, + echExt: []byte{254}, + expectRemoved: false, + }, + { + name: "echInner true, ech ext present, inner ech sentinel", + echInner: true, + echExt: []byte{byte(innerECHExt)}, + expectRemoved: false, + }, + { + name: "echInner false, no ech ext", + echInner: false, + expectRemoved: false, + }, + { + name: "echInner false, ech ext present", + echInner: false, + echExt: []byte{254}, + expectRemoved: true, + }, + { + name: "echInner false, ech ext present, inner ech sentinel", + echInner: false, + echExt: []byte{byte(innerECHExt)}, + expectRemoved: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ch := (&clientHelloMsg{}).Generate(r, 0).Interface().(*clientHelloMsg) + + ch.pskBinders = [][]byte{[]byte("test")} + ch.pskIdentities = []pskIdentity{{label: []byte("test")}} + ch.encryptedClientHello = tc.echExt + + b, err := ch.marshalMsg(tc.echInner) + if err != nil { + t.Fatal(err) + } + var rch clientHelloMsg + if !rch.unmarshal(b) { + t.Fatal("Failed to unmarshal ClientHello") + } + + if tc.expectRemoved { + if rch.pskIdentities != nil { + t.Error("expected PSK identities to be removed") + } + if rch.pskBinders != nil { + t.Error("expected PSK binders to be removed") + } + } else { + if rch.pskIdentities == nil { + t.Error("expected PSK identities to be present") + } + if rch.pskBinders == nil { + t.Error("expected PSK binders to be present") + } + } + }) + } + +} diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go index 86322390862137..655f58b747dbca 100644 --- a/src/crypto/tls/tls_test.go +++ b/src/crypto/tls/tls_test.go @@ -2379,6 +2379,7 @@ func TestECH(t *testing.T) { clientConfig.RootCAs.AddCert(secretCert) clientConfig.RootCAs.AddCert(publicCert) clientConfig.EncryptedClientHelloConfigList = echConfigList + clientConfig.ClientSessionCache = NewLRUClientSessionCache(2) serverConfig.InsecureSkipVerify = false serverConfig.Rand = rand.Reader serverConfig.Time = nil From f9ef7f55988f03afeb3b8354367d0fa8d053683d Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 26 May 2026 13:59:05 -0700 Subject: [PATCH 11/12] [release-branch.go1.26] os: properly handle trailing slashes in paths in Root This change fixes a significant mechanism by which operations in a Root can escape the root. The implementation of Root on platforms supporting the openat family of functions assumed that openat(parent, "f/", O_NOFOLLOW) would not resolve symlinks in "f". This is not correct; the trailing slash causes f to be resolved. This permits Root operations to escape when the target filename ends in a slash and the target is a symlink to a directory outside the root. This does not permit directly accessing non-directory files outside a root, since the trailing slash adds a requirement that the target be a directory. However, under some circumstances an attacker might exploit this flaw to access non-directory files outside a root, for example by first renaming a directory outside the root to a location within it and then accessing files within that directory. This change adjusts Root's handling of slash-terminated paths. Trailing slashes are removed from the path at the start of an operation, and the presence of slashes is tracked as a boolean. Slashes are never reattached to a path component. In addition, the doInRoot helper function now automatically handles trailing slashes in a POSIX-compatible fashion. When a path ends in one or more slashes: - symlinks in the final component are resolved; and - the final path component after symlink resolutions must reference a directory. This change also adds a new sets of tests to exercise Root's behavior in a wider variety of circumstances. These tests run through a matrix of file configurations, such as: - path "target", a regular file - path "dir/../target", a directory - path "target/", a symlink to "dir/../target/", which does not exist - etc. These tests execute Root operations and the corresponding unrooted operation, validate specific expected results for some configurations, and verify that the rooted and unrooted versions of the operation produce the same result. Thanks to Mundur (https://github.com/M0nd0R) for reporting this issue. Fixes #79005 Fixes CVE-2026-39822 Change-Id: I34072ab63f2367baf236592f11143f4e6a6a6964 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/4740 Reviewed-by: Neal Patel Reviewed-by: Roland Shoemaker Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/4840 Reviewed-on: https://go-review.googlesource.com/c/go/+/797661 Reviewed-by: Junyang Shao Auto-Submit: Gopher Robot TryBot-Bypass: Gopher Robot Reviewed-by: David Chase --- src/os/export_test.go | 1 + src/os/root.go | 18 +- src/os/root_js.go | 17 +- src/os/root_noopenat.go | 5 + src/os/root_openat.go | 163 ++++- src/os/root_test.go | 1443 +++++++++++++++++++++++++++++++++++++- src/os/root_unix.go | 39 +- src/os/root_unix_test.go | 5 +- src/os/root_windows.go | 56 +- 9 files changed, 1627 insertions(+), 120 deletions(-) diff --git a/src/os/export_test.go b/src/os/export_test.go index bea38c905a673f..01bdf8cea0538f 100644 --- a/src/os/export_test.go +++ b/src/os/export_test.go @@ -9,6 +9,7 @@ package os var Atime = atime var ErrWriteAtInAppendMode = errWriteAtInAppendMode var ErrPatternHasSeparator = errPatternHasSeparator +var ErrPathEscapes = errPathEscapes func init() { checkWrapErr = true diff --git a/src/os/root.go b/src/os/root.go index d759727ce7a89b..80e655e4fbdac2 100644 --- a/src/os/root.go +++ b/src/os/root.go @@ -297,20 +297,20 @@ func (r *Root) logStat(name string) { // // "." components are removed, except in the last component. // -// Path separators following the last component are returned in suffixSep. -func splitPathInRoot(s string, prefix, suffix []string) (_ []string, suffixSep string, err error) { +// endsInSlash reports whether the path ends in one or more slashes. +func splitPathInRoot(s string, prefix, suffix []string) (_ []string, endsInSlash bool, err error) { if len(s) == 0 { - return nil, "", errors.New("empty path") + return nil, false, errors.New("empty path") } if IsPathSeparator(s[0]) { - return nil, "", errPathEscapes + return nil, false, errPathEscapes } if runtime.GOOS == "windows" { // Windows cleans paths before opening them. s, err = rootCleanPath(s, prefix, suffix) if err != nil { - return nil, "", err + return nil, false, err } prefix = nil suffix = nil @@ -332,8 +332,10 @@ func splitPathInRoot(s string, prefix, suffix []string) (_ []string, suffixSep s } if j == len(s) { // If this is the last path component, - // preserve any trailing path separators. - suffixSep = s[partEnd:] + // check for any trailing path separators. + if len(s[partEnd:]) > 0 { + endsInSlash = true + } break } if parts[len(parts)-1] == "." { @@ -347,7 +349,7 @@ func splitPathInRoot(s string, prefix, suffix []string) (_ []string, suffixSep s parts = parts[:len(parts)-1] } parts = append(parts, suffix...) - return parts, suffixSep, nil + return parts, endsInSlash, nil } // FS returns a file system (an fs.FS) for the tree of files in the root. diff --git a/src/os/root_js.go b/src/os/root_js.go index 56a37dafe1626e..b201f1c406c568 100644 --- a/src/os/root_js.go +++ b/src/os/root_js.go @@ -33,7 +33,7 @@ func checkPathEscapesInternal(r *Root, name string, lstat bool) error { if r.root.closed.Load() { return ErrClosed } - parts, suffixSep, err := splitPathInRoot(name, nil, nil) + parts, endsInSlash, err := splitPathInRoot(name, nil, nil) if err != nil { return err } @@ -63,10 +63,9 @@ func checkPathEscapesInternal(r *Root, name string, lstat bool) error { part := parts[i] if i == len(parts)-1 { - if lstat { + if lstat && !endsInSlash { break } - part += suffixSep } next := joinPath(base, part) @@ -86,18 +85,12 @@ func checkPathEscapesInternal(r *Root, name string, lstat bool) error { if symlinks > rootMaxSymlinks { return errors.New("too many symlinks") } - newparts, newSuffixSep, err := splitPathInRoot(link, parts[:i], parts[i+1:]) + newparts, newEndsInSlash, err := splitPathInRoot(link, parts[:i], parts[i+1:]) if err != nil { return err } - if i == len(parts) { - // suffixSep contains any trailing path separator characters - // in the link target. - // If we are replacing the remainder of the path, retain these. - // If we're replacing some intermediate component of the path, - // ignore them, since intermediate components must always be - // directories. - suffixSep = newSuffixSep + if i == len(parts)-1 && newEndsInSlash { + endsInSlash = true } parts = newparts continue diff --git a/src/os/root_noopenat.go b/src/os/root_noopenat.go index 59f1abe91b0b2e..228b580db41be9 100644 --- a/src/os/root_noopenat.go +++ b/src/os/root_noopenat.go @@ -187,6 +187,11 @@ func rootRemove(r *Root, name string) error { } func rootRemoveAll(r *Root, name string) error { + // Consistency with os.RemoveAll: Strip trailing /s from the name, + // so RemoveAll("not_a_directory/") succeeds. + for len(name) > 0 && IsPathSeparator(name[len(name)-1]) { + name = name[:len(name)-1] + } if endsWithDot(name) { // Consistency with os.RemoveAll: Return EINVAL when trying to remove . return &PathError{Op: "RemoveAll", Path: name, Err: syscall.EINVAL} diff --git a/src/os/root_openat.go b/src/os/root_openat.go index 83bde5ef14160d..d7c9334ac1919c 100644 --- a/src/os/root_openat.go +++ b/src/os/root_openat.go @@ -7,6 +7,7 @@ package os import ( + "io/fs" "runtime" "slices" "sync" @@ -66,7 +67,7 @@ func (r *root) Name() string { } func rootChmod(r *Root, name string, mode FileMode) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, chmodat(parent, name, mode) }) if err != nil { @@ -76,7 +77,7 @@ func rootChmod(r *Root, name string, mode FileMode) error { } func rootChown(r *Root, name string, uid, gid int) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, chownat(parent, name, uid, gid) }) if err != nil { @@ -86,27 +87,36 @@ func rootChown(r *Root, name string, uid, gid int) error { } func rootLchown(r *Root, name string, uid, gid int) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, lchownat(parent, name, uid, gid) }) if err != nil { return &PathError{Op: "lchownat", Path: name, Err: err} } - return err + return nil } func rootChtimes(r *Root, name string, atime time.Time, mtime time.Time) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, chtimesat(parent, name, atime, mtime) }) if err != nil { return &PathError{Op: "chtimesat", Path: name, Err: err} } - return err + return nil } func rootMkdir(r *Root, name string, perm FileMode) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + flags := uint(doInRootCreatingDirectory) + switch runtime.GOOS { + case "linux", "windows": + // These platforms do not follow "symlink" on "mkdir symlink/". + // (POSIX.1-2024 4.16 says that the trailing slash should cause + // resolution to follow the symlink, but we're trying to match + // platform semantics, not implement POSIX.) + flags = doInRootNoHandleTerminalSlash + } + _, err := doInRoot(r, name, flags, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, mkdirat(parent, name, perm) }) if err != nil { @@ -140,7 +150,7 @@ func rootMkdirAll(r *Root, fullname string, perm FileMode) error { panic("unreachable") } // openLastComponentFunc opens the last path component. - openLastComponentFunc := func(parent sysfdType, name string) (struct{}, error) { + openLastComponentFunc := func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { err := mkdirat(parent, name, perm) if err == syscall.EEXIST { mode, e := modeAt(parent, name) @@ -167,7 +177,7 @@ func rootMkdirAll(r *Root, fullname string, perm FileMode) error { } return struct{}{}, &PathError{Op: "mkdirat", Err: err} } - _, err := doInRoot(r, fullname, openDirFunc, openLastComponentFunc) + _, err := doInRoot(r, fullname, 0, openDirFunc, openLastComponentFunc) if err != nil { if _, ok := err.(*PathError); !ok { err = &PathError{Op: "mkdirat", Path: fullname, Err: err} @@ -177,7 +187,7 @@ func rootMkdirAll(r *Root, fullname string, perm FileMode) error { } func rootReadlink(r *Root, name string) (string, error) { - target, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (string, error) { + target, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (string, error) { return readlinkat(parent, name) }) if err != nil { @@ -187,7 +197,7 @@ func rootReadlink(r *Root, name string) (string, error) { } func rootRemove(r *Root, name string) error { - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, removeat(parent, name) }) if err != nil { @@ -206,7 +216,7 @@ func rootRemoveAll(r *Root, name string) error { // Consistency with os.RemoveAll: Return EINVAL when trying to remove . return &PathError{Op: "RemoveAll", Path: name, Err: syscall.EINVAL} } - _, err := doInRoot(r, name, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, name, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, removeAllFrom(parent, name) }) if IsNotExist(err) { @@ -219,8 +229,30 @@ func rootRemoveAll(r *Root, name string) error { } func rootRename(r *Root, oldname, newname string) error { - _, err := doInRoot(r, oldname, nil, func(oldparent sysfdType, oldname string) (struct{}, error) { - _, err := doInRoot(r, newname, nil, func(newparent sysfdType, newname string) (struct{}, error) { + _, err := doInRoot(r, oldname, 0, nil, func(oldparent sysfdType, oldname string, oldEndsInSlash bool) (struct{}, error) { + flags := uint(doInRootCreatingDirectory) + if runtime.GOOS == "windows" { + flags = doInRootNoHandleTerminalSlash + } + _, err := doInRoot(r, newname, flags, nil, func(newparent sysfdType, newname string, newEndsInSlash bool) (struct{}, error) { + if runtime.GOOS != "windows" && newEndsInSlash { + oldMode, err := modeAt(oldparent, oldname) + if err != nil { + return struct{}{}, err + } + if oldMode.Type() != fs.ModeDir { + return struct{}{}, syscall.ENOTDIR + } + } + // Same checks as applied by rename (in file_unix.go): + fi, err := lstatat(newparent, newname) + if err == nil && fi.IsDir() { + if ofi, err := lstatat(oldparent, oldname); err != nil { + return struct{}{}, err + } else if newname == oldname || !SameFile(fi, ofi) { + return struct{}{}, syscall.EEXIST + } + } return struct{}{}, renameat(oldparent, oldname, newparent, newname) }) return struct{}{}, err @@ -232,8 +264,13 @@ func rootRename(r *Root, oldname, newname string) error { } func rootLink(r *Root, oldname, newname string) error { - _, err := doInRoot(r, oldname, nil, func(oldparent sysfdType, oldname string) (struct{}, error) { - _, err := doInRoot(r, newname, nil, func(newparent sysfdType, newname string) (struct{}, error) { + _, err := doInRoot(r, oldname, 0, nil, func(oldparent sysfdType, oldname string, oldEndsInSlash bool) (struct{}, error) { + flags := uint(0) + if runtime.GOOS == "windows" { + // Windows doesn't pay attention to trailing slashes in the link target. + flags = doInRootNoHandleTerminalSlash + } + _, err := doInRoot(r, newname, flags, nil, func(newparent sysfdType, newname string, newEndsInSlash bool) (struct{}, error) { return struct{}{}, linkat(oldparent, oldname, newparent, newname) }) return struct{}{}, err @@ -244,31 +281,58 @@ func rootLink(r *Root, oldname, newname string) error { return err } +// Flags for doInRoot. +const ( + // doInRootNoHandleTerminalSlash prevents doInRoot from applying special handling + // for paths which end in one or more slashes. + doInRootNoHandleTerminalSlash = 1 << iota + + // doInRootCreatingDirectory indicates that the operation is creating a directory. + // When a path ends in /, the last path component may name a file which does not exist. + doInRootCreatingDirectory + + // doInRootAlwaysResolveTerminalSlash causes doInRoot to resolve symlinks in the last + // path component when a path ends in /, even on Windows. For example, this causes + // doInRoot to resolve "symlink/" as the link target of "symlink". + // + // POSIX path operations resolve symlinks in this case. + // Most Windows operations do not. + // This flag enforces the POSIX behavior. + doInRootAlwaysResolveTerminalSlash +) + // doInRoot performs an operation on a path in a Root. // // It calls f with the FD or handle for the directory containing the last -// path element, and the name of the last path element. +// path element, the name of the last path element (not including slashes), +// and a boolean indicating whether the original path ended in one or more slashes. // // For example, given the path a/b/c it calls f with the FD for a/b and the name "c". // +// It applies special handling for paths ending in a slash: When a path ends in a slash +// (for example "a/b/"), doInRoot will check the final component ("b") before calling f. +// If the final component is a symlink, doInRoot will resolve it. +// If the final component is neither a symlink nor a directory, doInRoot will return ENOTDIR. +// This behavior may be disabled by passing the doInRootNoHandleTerminalSlash flag. +// // If openDirFunc is non-nil, it is called to open intermediate path elements. // For example, given the path a/b/c openDirFunc will be called to open a and a/b in turn. // // f or openDirFunc may return errSymlink to indicate that the path element is a symlink // which should be followed. Note that this can result in f being called multiple times -// with different names. For example, give the path "link" which is a symlink to "target", +// with different names. For example, given the path "link" which is a symlink to "target", // f is called with the path "link", returns errSymlink("target"), and is called again with // the path "target". // // If f or openDirFunc return a *PathError, doInRoot will set PathError.Path to the // full path which caused the error. -func doInRoot[T any](r *Root, name string, openDirFunc func(parent sysfdType, name string) (sysfdType, error), f func(parent sysfdType, name string) (T, error)) (ret T, err error) { +func doInRoot[T any](r *Root, name string, flags uint, openDirFunc func(parent sysfdType, name string) (sysfdType, error), f func(parent sysfdType, name string, endsInSlash bool) (T, error)) (ret T, err error) { if err := r.root.incref(); err != nil { return ret, err } defer r.root.decref() - parts, suffixSep, err := splitPathInRoot(name, nil, nil) + parts, endsInSlash, err := splitPathInRoot(name, nil, nil) if err != nil { return ret, err } @@ -332,15 +396,43 @@ Loop: } if i == len(parts)-1 { + err = nil + if endsInSlash && flags&doInRootNoHandleTerminalSlash == 0 { + var fi FileInfo + fi, err = lstatat(dirfd, parts[i]) + switch { + case IsNotExist(err) && flags&doInRootCreatingDirectory != 0: + // The path ends in a slash, the last path component + // does not exist, and we creating a directory. + // This is fine. + err = nil + case err != nil: + return + case fi.Mode().Type() == fs.ModeDir: + case fi.Mode().Type() == fs.ModeSymlink: + if runtime.GOOS != "windows" || flags&doInRootAlwaysResolveTerminalSlash != 0 { + err = checkSymlink(dirfd, parts[i], syscall.ENOTDIR) + } else { + if !isDirectoryLink(fi) { + err = syscall.ENOTDIR + } + } + default: + err = syscall.ENOTDIR + return + } + } + // This is the last path element. // Call f to decide what to do with it. // If f returns errSymlink, this element is a symlink // which should be followed. - // suffixSep contains any trailing separator characters - // which we rejoin to the final part at this time. - ret, err = f(dirfd, parts[i]+suffixSep) + // suffixSep contains any trailing separator characters. if err == nil { - return + ret, err = f(dirfd, parts[i], endsInSlash) + if err == nil { + return + } } } else { var fd sysfdType @@ -360,18 +452,15 @@ Loop: if symlinks > rootMaxSymlinks { return ret, syscall.ELOOP } - newparts, newSuffixSep, err := splitPathInRoot(string(e), parts[:i], parts[i+1:]) + lastPart := i == len(parts)-1 + newparts, newEndsInSlash, err := splitPathInRoot(string(e), parts[:i], parts[i+1:]) if err != nil { return ret, err } - if i == len(parts)-1 { - // suffixSep contains any trailing path separator characters - // in the link target. - // If we are replacing the remainder of the path, retain these. - // If we're replacing some intermediate component of the path, - // ignore them, since intermediate components must always be - // directories. - suffixSep = newSuffixSep + if lastPart && newEndsInSlash { + // If a link target in the final path component ends in a slash, + // then the path now ends in a slash. + endsInSlash = true } if len(newparts) < i || !slices.Equal(parts[:i], newparts[:i]) { // Some component in the path which we have already traversed @@ -399,6 +488,14 @@ Loop: } } +func modeAt(parent sysfdType, name string) (FileMode, error) { + fi, err := lstatat(parent, name) + if err != nil { + return 0, err + } + return fi.Mode(), nil +} + // errSymlink reports that a file being operated on is actually a symlink, // and the target of that symlink. type errSymlink string diff --git a/src/os/root_test.go b/src/os/root_test.go index 71ce5e559a2dba..1678d200eb2dd9 100644 --- a/src/os/root_test.go +++ b/src/os/root_test.go @@ -7,10 +7,12 @@ package os_test import ( "bytes" "errors" + "flag" "fmt" "internal/testenv" "io" "io/fs" + "iter" "net" "os" "path" @@ -55,7 +57,7 @@ func testMaybeRooted(t *testing.T, f func(t *testing.T, r *os.Root)) { // // makefs calls t.Skip if the layout contains features not supported by the current GOOS. func makefs(t *testing.T, fs []string) string { - root := path.Join(t.TempDir(), "ROOT") + root := filepath.Join(t.TempDir(), "ROOT") if err := os.Mkdir(root, 0o777); err != nil { t.Fatal(err) } @@ -203,6 +205,22 @@ var rootTestCases = []rootTest{{ }, open: "link/target", target: "dir/target", +}, { + name: "slash after symlink to file", + fs: []string{ + "link => ../ROOT/target", + }, + open: "link/", + target: "target", + wantError: true, +}, { + name: "slash after symlink to dir", + fs: []string{ + "link => ../ROOT/target", + "target/", + }, + open: "link/", + wantError: true, }, { name: "symlink dotdot dotdot slash", fs: []string{ @@ -244,8 +262,10 @@ var rootTestCases = []rootTest{{ open: "../", wantError: true, }, { - name: "path with dotdot dotdot slash", - fs: []string{}, + name: "path with dotdot dotdot slash", + fs: []string{ + "a/", + }, open: "a/../../", wantError: true, }, { @@ -521,13 +541,10 @@ func TestRootMkdir(t *testing.T) { for _, test := range rootTestCases { test.run(t, func(t *testing.T, target string, root *os.Root) { wantError := test.wantError - if !wantError { - fi, err := os.Lstat(filepath.Join(root.Name(), test.open)) - if err == nil && fi.Mode().Type() == fs.ModeSymlink { - // This case is trying to mkdir("some symlink"), - // which is an error. - wantError = true - } + if test.ltarget != "" { + // This case is trying to mkdir("some symlink"), + // which is an error (but not an escape). + wantError = true } err := root.Mkdir(test.open, 0o777) @@ -555,13 +572,10 @@ func TestRootMkdirAll(t *testing.T) { for _, test := range rootTestCases { test.run(t, func(t *testing.T, target string, root *os.Root) { wantError := test.wantError - if !wantError { - fi, err := os.Lstat(filepath.Join(root.Name(), test.open)) - if err == nil && fi.Mode().Type() == fs.ModeSymlink { - // This case is trying to mkdir("some symlink"), - // which is an error. - wantError = true - } + if test.ltarget != "" { + // This case is trying to mkdir("some symlink"), + // which is an error (but not an escape). + wantError = true } err := root.Mkdir(test.open, 0o777) @@ -667,6 +681,15 @@ func TestRootRemoveDirectory(t *testing.T) { func TestRootRemoveAll(t *testing.T) { for _, test := range rootTestCases { test.run(t, func(t *testing.T, target string, root *os.Root) { + if strings.HasSuffix(test.open, "/") { + // The test is removing a file with a trailing /. + // RemoveAll ignores trailing /s + // If the file is a symlink, it will remove the symlink. + fullname := filepath.Join(root.Name(), test.open) + if st, err := os.Lstat(fullname); err == nil && st.Mode().Type() == fs.ModeSymlink { + test.ltarget = test.open + } + } wantError := test.wantError if test.ltarget != "" { // Remove doesn't follow symlinks in the final path component, @@ -945,6 +968,15 @@ func testRootMoveTo(t *testing.T, rename bool) { t.Fatal(err) } + if runtime.GOOS == "windows" && strings.HasSuffix(test.open, "/") { + // Windows will ignore trailing slashes in the rename/link target. + p := strings.TrimSuffix(test.open, "/") + st, err := root.Lstat(p) + if err == nil && st.Mode().Type() == fs.ModeSymlink { + test.ltarget = p + } + } + target = test.target wantError := test.wantError if test.ltarget != "" { @@ -1106,6 +1138,16 @@ var rootConsistencyTestCases = []rootConsistencyTest{{ "link => target", }, open: "link/", + check: func(t *testing.T) { + if runtime.GOOS == "linux" && strings.HasPrefix(t.Name(), "TestRootConsistencyRename/") { + // Linux does not resolve "symlink" in rename("symlink/", "target"). + t.Skip("known inconsistency on linux") + } + if strings.HasPrefix(t.Name(), "TestRootConsistencyRemoveAll/") { + // Root.RemoveAll and os.RemoveAll are not always consistent here. + t.Skip("known inconsistency in RemoveAll") + } + }, }, { name: "symlink slash dot", fs: []string{ @@ -1113,17 +1155,6 @@ var rootConsistencyTestCases = []rootConsistencyTest{{ "link => target", }, open: "link/.", -}, { - name: "file symlink slash", - fs: []string{ - "target", - "link => target", - }, - open: "link/", - detailedErrorMismatch: func(t *testing.T) bool { - // os.Create returns ENOTDIR or EISDIR depending on the platform. - return runtime.GOOS == "js" - }, }, { name: "unresolved symlink", fs: []string{ @@ -1193,13 +1224,19 @@ var rootConsistencyTestCases = []rootConsistencyTest{{ return runtime.GOOS == "windows" }, check: func(t *testing.T) { - if runtime.GOOS == "windows" && strings.HasPrefix(t.Name(), "TestRootConsistencyRemoveAll/") { - // Root.RemoveAll notices that a/ is not a directory, - // and returns success. - // os.RemoveAll tries to open a/ and fails because - // it is not a regular file. - // The inconsistency here isn't worth fixing, so just skip this test. - t.Skip("known inconsistency on windows") + if strings.HasPrefix(t.Name(), "TestRootConsistencyRemoveAll/") { + switch runtime.GOOS { + case "windows": + // Root.RemoveAll notices that a/ is not a directory, + // and returns success. + // os.RemoveAll tries to open a/ and fails because + // it is not a regular file. + // The inconsistency here isn't worth fixing, so just skip this test. + t.Skip("known inconsistency on windows") + case "js": + // GOOS=js behavior varies with what the underlying OS is. + t.Skip("known inconsistency with GOOS=js") + } } }, }, { @@ -2057,3 +2094,1339 @@ func TestRootNoLstat(t *testing.T) { } }) } + +// A rootMultiTest is state for testing an os.Root operation in one configuration among many. +// Each execution of a rootMultiTest varies in several ways: +// +// - With or without an *os.Root, to check consistency between root/non-root operations. +// - With a target that may be a file, directory, symlink, or entirely absent. +// - With various paths referencing the target: "target", "DIR/../target", etc. +// - When the target is a symlink, with various link target paths. +// +// For example, a single test execution might be: +// In an *os.Root, copy "source" to "DIR/../target". +// "source" is a file, and "target" is a symlink to "../ROOT/s_target". "s_target" is a directory. +// (In this case, we expect the test to fail due to the path escape in the symlink.) +type rootMultiTest struct { + // dir is the directory containing the test. + // dir will always contain a directory named "ROOT" + // and a subdir named "ROOT/DIR". + dir string + + // root is the *Root for the test. May be nil. + root *os.Root + + // source and target are files acted on by the test. + // target is always set; source is only set for tests which request two files. + source testFileDesc + target testFileDesc + + // sourcePath and targetPath are the paths which should be used to acceess + // the source/target. + sourcePath string + targetPath string + + sourceInfo os.FileInfo + targetInfo os.FileInfo + + // op is the operation being performed, used for reporting errors. + op string +} + +var testVerbose = flag.Bool("verbose", false, "verbose") + +// A rootMultiTest function may return this error to disable +// the check that in-root and out-of-root functions have the same outcome. +var errSkipRootConsistencyCheck = errors.New("skip root consistency check") + +// runRootMultiTest runs f in a variety of configurations. +// See above. +func runRootMultiTest(t *testing.T, f func(*testing.T, *rootMultiTest) (string, error)) { + for target := range allTestFileDescs() { + t.Run(target.String(), func(t *testing.T) { + var source testFileDesc // unused + runRootMultiTestDescs(t, source, target, f) + }) + } +} + +// runRootMultiTest2 runs f in a variety of configurations, +// with both source and target files. +// See above. +func runRootMultiTest2(t *testing.T, f func(*testing.T, *rootMultiTest) (string, error)) { + // A "simple" desc is one which contains only direct references. + // When not running the comprehensive (but slow) set of test variations, + // we only test variations where at least one of source and target is simple. + isSimple := func(desc testFileDesc) bool { + if desc.ref.template != "BASE" { + return false + } + if desc.kind == testFileSymlink && desc.target.ref.template != "BASE" { + return false + } + return true + } + for source := range allTestFileDescs() { + for target := range allTestFileDescs() { + if !*rootComprehensive && !isSimple(source) && !isSimple(target) { + continue + } + name := fmt.Sprintf("%s_to_%s", source, target) + t.Run(name, func(t *testing.T) { + runRootMultiTestDescs(t, source, target, f) + }) + } + } +} + +// setOp sets the operation performed by the test (logged in errors). +// +// This currently assumes the operation will be a method of os.Root and a function in os +// (e.g., root.Open/os.Open). +func (test *rootMultiTest) setOp(format string, a ...any) { + if test.root != nil { + test.op = "root." + } else { + test.op = "os." + } + test.op += fmt.Sprintf(format, a...) +} + +var errAny = errors.New("any error") + +func (test *rootMultiTest) errorf(t *testing.T, format string, args ...any) { + t.Errorf("%v:", test.op) + t.Fatalf(" "+format, args...) +} + +// wantError tests whether got matches want. +// If want is errAny, got may be any non-nil error. +func (test *rootMultiTest) wantError(t *testing.T, got, want error) { + t.Helper() + if errors.Is(got, want) || (got != nil && want == errAny) { + return + } + t.Fatalf("%v:\ngot error: %v\nwant error: %v", test.op, got, want) +} + +func runRootMultiTestDescs(t *testing.T, source, target testFileDesc, f func(*testing.T, *rootMultiTest) (string, error)) { + rootTest := newRootTest(t, source, target, true) + osTest := newRootTest(t, source, target, false) + + initialContent := dirTreeContents(t, rootTest.dir) + t.Cleanup(func() { + if t.Failed() { + t.Log("Initial directory contents:") + for _, line := range initialContent { + t.Logf(" %v", line) + } + } + }) + + rootResult, rootErr := f(t, rootTest) + + if runtime.GOOS == "darwin" { + // Darwin appears to have a kernel bug which causes restrictions on paths + // with a trailing / to not be applied during uncached path lookups. + // These restrictions are applied during cached lookups, so the results + // of operating on /-suffixed paths are inconsistent. + // + // An example of this Darwin behavior (as of 25.4.0) is: + // $ mkdir -p test/dir + // $ echo hello > test/file + // $ ln -s dir/../file test/link + // $ cat test/link/ + // hello + // $ cat test/link/ + // cat: test/link/: Not a directory + // + // Since Darwin isn't consistent with itself, we can't verify that we're + // consistent with it. + if rootTest.source.anySlashSuffix() || rootTest.target.anySlashSuffix() { + return + } + } + + if runtime.GOOS == "wasip1" || runtime.GOOS == "js" { + // WASI runtimes don't have any consistent behavior for handling paths with + // a trailing /, so skip consistency tests for these paths. + if rootTest.source.anySlashSuffix() || rootTest.target.anySlashSuffix() { + return + } + } + + osResult, osErr := f(t, osTest) + + t.Cleanup(func() { + if t.Failed() || !*testVerbose { + return + } + rootContent := dirTreeContents(t, rootTest.dir) + osContent := dirTreeContents(t, osTest.dir) + t.Log("Initial directory contents:") + for _, line := range initialContent { + t.Logf(" %v", line) + } + t.Logf("%v:", rootTest.op) + t.Logf(" result: %v", rootResult) + t.Logf(" error: %v", rootErr) + for _, line := range rootContent { + t.Logf(" %v", line) + } + t.Logf("%v:", osTest.op) + t.Logf(" result: %v", osResult) + t.Logf(" error: %v", osErr) + for _, line := range osContent { + t.Logf(" %v", line) + } + }) + + if errors.Is(rootErr, os.ErrPathEscapes) { + // os.Root forbids this operation (and is therefore not consistent with + // the non-root version). + return + } + + if rootErr == errSkipRootConsistencyCheck || osErr == errSkipRootConsistencyCheck { + return + } + + // Consistency check: Performing the same operation in and out of a root + // should produce the same results. + if rootResult != osResult { + t.Errorf("inconsistent results in/out of root") + t.Errorf("%v:", rootTest.op) + t.Errorf(" result: %v", rootResult) + t.Errorf("%v:", osTest.op) + t.Errorf(" result: %v", osResult) + } + if (rootErr == nil) != (osErr == nil) { + t.Errorf("inconsistent errors in/out of root") + t.Errorf("%v:", rootTest.op) + t.Errorf(" error: %v", rootErr) + t.Errorf("%v:", osTest.op) + t.Errorf(" error: %v", osErr) + } + + // Filesystem consistency check: Same files in the same places. + rootContent := dirTreeContents(t, rootTest.dir) + osContent := dirTreeContents(t, osTest.dir) + if !slices.Equal(rootContent, osContent) { + t.Errorf("inconsistent filesystem after running in/out of root") + t.Errorf("%v:", rootTest.op) + for _, line := range rootContent { + t.Errorf(" %v", line) + } + t.Errorf("%v:", osTest.op) + for _, line := range osContent { + t.Errorf(" %v", line) + } + } +} + +func newRootTest(t *testing.T, source, target testFileDesc, inRoot bool) *rootMultiTest { + dir := makefs(t, []string{ + "DIR/", + }) + var root *os.Root + if inRoot { + var err error + root, err = os.OpenRoot(dir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + root.Close() + }) + } + test := &rootMultiTest{ + dir: dir, + root: root, + source: source, + target: target, + } + createFile := func(name string, desc testFileDesc) (path string, fi os.FileInfo) { + if desc.kind == testFileUnused { + return "", nil + } + fi = desc.create(t, dir, name, name) + path = desc.ref.path(dir, name) + if !inRoot && !filepath.IsAbs(path) { + path = dir + "/" + path + } + return path, fi + } + test.sourcePath, test.sourceInfo = createFile("source", source) + test.targetPath, test.targetInfo = createFile("target", target) + return test +} + +// testFileKind is a kind of file. +type testFileKind int + +const ( + testFileUnused = testFileKind(iota) + testFileAbsent // file does not exist + testFileFile // regular file + testFileDir // directory + testFileSymlink // symlink + testFileMax + + // testFileError represents a path which fails during resolution, + // such as "a/b" where "a" does not exist. + testFileError +) + +func (kind testFileKind) String() string { + switch kind { + case testFileUnused: + return "unused" + case testFileAbsent: + return "absent" + case testFileFile: + return "file" + case testFileDir: + return "dir" + case testFileSymlink: + return "symlink" + case testFileError: + return "error" + default: + return fmt.Sprintf("testFileKind(%d)", kind) + } +} + +// testFileRef is a kind of reference to a file. +// +// Many path names can refer to the same file: f, ./f, /abs/path/to/f, somedir/../f, etc. +// A testFileRef describes some form of reference. +type testFileRef struct { + // name is the name of the reference (not the file name). + // These are a bit cryptic to keep test names short: + // s (/ slash), p (.. parent), b (base), d (directory), r (root) + name string + + // template is a template path. + // + // templates assume that the file is contained in a directory named "ROOT", + // and that "ROOT/DIR" exists and is a directory. + // + // The string BASE in the template may be replaced with the file's basename. + // + // Absolute path templates start with /ROOT. + template string + + // escapes indicates whether the path escapes the current directory. + escapes bool +} + +var testFileRefs = []testFileRef{ + {escapes: false, name: "b", template: "BASE"}, + {escapes: false, name: "bs", template: "BASE/"}, + {escapes: false, name: "dpb", template: "DIR/../BASE"}, + {escapes: false, name: "dpbs", template: "DIR/../BASE/"}, + {escapes: true, name: "prb", template: "../ROOT/BASE"}, + {escapes: true, name: "prbs", template: "../ROOT/BASE/"}, + {escapes: true, name: "srb", template: "/ROOT/BASE"}, + {escapes: true, name: "srbs", template: "/ROOT/BASE/"}, +} + +// testFileLimitedRefs is a smaller set of references which do not exercise path escapes +// (see allTestFileDescs). +var testFileLimitedRefs = testFileRefs[0:2] + +// path creates a path using the template. +// +// dir is the absolute path to the root directory (which must be named "ROOT"). +// base is the name of the target file within the root directory. +func (ref testFileRef) path(dir, base string) string { + p := ref.template + p = strings.ReplaceAll(p, "BASE", base) + if trim, ok := strings.CutPrefix(p, "/ROOT"); ok { + p = dir + trim + } + return p +} + +// hasSlashSuffix reports whether the file reference ends in a /. +func (ref testFileRef) hasSlashSuffix() bool { + return strings.HasSuffix(ref.template, "/") +} + +// testFileDesc is a description of a type of file, combining the kind and reference type. +// +// Some sample testFileDescs: +// - "name", a plain file. +// - "DIR/../name", a directory +// - "name/", where name is a symlink to "DIR/../target/", where target is a plain file. +type testFileDesc struct { + kind testFileKind + ref testFileRef + target *testFileDesc // symlink target, nil when kind is not testFileSymlink +} + +var rootComprehensive = flag.Bool("root_comprehensive", false, + "run many more os.Root test variations (slow, uncertain value)") + +// allTestFileDescs returns an iterator over all the testFileDescs we use in tests. +func allTestFileDescs() iter.Seq[testFileDesc] { + // A testFileDesc contains a reference type ("name", "d/../name", "../r/name", etc.) and + // a file kind (file, directory, symlink, etc.). + // + // When the kind is symlink, the desc contains a reference type and file kind for + // the link target as well. We only exercise one level of symlink (although we + // could do more), so this means a testFileDesc effectively contains four axes of + // variation: ref, kind, symlink ref, symlink kind. + // + // For example: + // + // - "name" is a file + // - "d/../name" is a directory + // - "name" is a symlink to "name2" which is a file + // - "d/../name" is a symlink to "d/../name2" which is a directory + // - etc. + // + // It is feasible to test every possible variation of these four axes, + // but this is quite a few tests and gets quite slow. So by default we exclude + // some variations. We test: + // + // - every reference to every kind, except symlink + // - direct and direct/ references to a symlink to every reference to a file + // - a direct reference to a symlink to a direct reference to every kind (except file) + // + // The full set of variations may be enabled with the -comprehensive_root_tests flag. + + return func(yield func(testFileDesc) bool) { + // Every type of reference to every type of file, except symlink. + for _, ref := range testFileRefs { + for kind := range testFileMax { + if kind == testFileUnused || kind == testFileSymlink { + continue + } + desc := testFileDesc{ + kind: kind, + ref: ref, + } + if !yield(desc) { + return + } + } + } + + // Unless we're being comprehensive, only direct references to symlinks. + refs := testFileRefs + if !*rootComprehensive { + refs = testFileLimitedRefs + } + for _, ref := range refs { + for linkKind := range testFileMax { + if linkKind == testFileUnused || linkKind == testFileSymlink { + continue + } + + linkRefs := testFileRefs + if !*rootComprehensive && linkKind != testFileFile && linkKind != testFileDir { + linkRefs = testFileLimitedRefs + } + for _, linkRef := range linkRefs { + desc := testFileDesc{ + kind: testFileSymlink, + ref: ref, + target: &testFileDesc{ + kind: linkKind, + ref: linkRef, + }, + } + if !yield(desc) { + return + } + } + } + } + } +} + +// String returns the target name. +// +// These are somewhat cryptic to keep test names short. +// For example, "bsSdpbD" is: +// +// bs - "BASE/" +// S - symlink +// dpb - "DIR/../BASE" +// D - directory +// +// So, open "file1/", where file1 is a symlink to "DIR/../file2", where file2 is a directory. +func (desc testFileDesc) String() string { + s := desc.ref.name + strings.ToUpper(desc.kind.String()[:1]) + if desc.kind == testFileSymlink { + s += desc.target.String() + } + return s +} + +// escapes reports whether accessing this file escapes the root, +// either because the file name escapes or because some element of a symlink chain escapes. +func (desc testFileDesc) escapes() bool { + if desc.ref.escapes { + return true + } + if desc.kind == testFileSymlink { + return desc.target.escapes() + } + return false +} + +func (desc testFileDesc) lescapes() bool { + if desc.ref.escapes { + return true + } + if runtime.GOOS == "windows" { + // On POSIX filesystems, a trailing slash at the end of a path causes + // symlinks in the last path component to be resolved. + // On Windows, a trailing slash does not cause symlink resolution. + return false + } + if desc.ref.hasSlashSuffix() && desc.kind == testFileSymlink { + return desc.target.escapes() + } + return false +} + +// finalKind reports the kind of the file after following all symlinks. +func (desc testFileDesc) finalKind() testFileKind { + if desc.kind == testFileSymlink { + return desc.target.finalKind() + } + return desc.kind +} + +func (desc testFileDesc) lfinalKind() testFileKind { + switch runtime.GOOS { + case "windows": + if desc.ref.hasSlashSuffix() && desc.kind == testFileSymlink && desc.target.kind != testFileDir { + return testFileError + } + default: + if desc.ref.hasSlashSuffix() && desc.kind == testFileSymlink { + return desc.target.finalKind() + } + } + return desc.kind +} + +func (desc testFileDesc) isError() bool { + if runtime.GOOS == "js" { + return false + } + var isError func(desc testFileDesc, hasSuffix bool) bool + isError = func(desc testFileDesc, hasSuffix bool) bool { + if desc.ref.escapes { + return false + } + if desc.ref.hasSlashSuffix() { + hasSuffix = true + } + switch desc.kind { + case testFileDir: + return false + case testFileSymlink: + if runtime.GOOS == "windows" && hasSuffix && desc.target.kind != testFileDir { + return true + } + return isError(*desc.target, hasSuffix) + default: + return hasSuffix + } + } + return isError(desc, false) +} + +func (desc testFileDesc) isSymlinkToDir() bool { + if desc.kind != testFileSymlink { + return false + } + if desc.ref.escapes { + return false + } + if desc.finalKind() == testFileDir { + return true + } + return false +} + +// anySlashSuffix reports whether any of the names in the file +// (either the initial name, or a symlink target) +// include a trailing /. +func (desc testFileDesc) anySlashSuffix() bool { + name := desc.ref.template + if len(name) > 0 && os.IsPathSeparator(name[len(name)-1]) { + return true + } + if desc.kind == testFileSymlink { + return desc.target.anySlashSuffix() + } + return false +} + +// anySlashSuffix reports whether the name of the file includes a trailing /. +func (desc testFileDesc) slashSuffix() bool { + name := desc.ref.template + if len(name) > 0 && os.IsPathSeparator(name[len(name)-1]) { + return true + } + return false +} + +// create creates the file(s) for this descriptor. +// +// dir is the test root directory. +// base is the base name of the file we will open within the root. +// (If there are symlinks, base is the start of the symlink chain.) +// +// Tests may create, delete, or move files, which makes it useful to have a way to identify +// and track the files that existed at the start of the test. The token parameter identifies +// which file we're creating. When symlinks are involved, the token is used in creating the +// final, non-symlink file. +func (desc testFileDesc) create(t *testing.T, dir, base, token string) (fi os.FileInfo) { + path := filepath.Join(dir, base) + switch desc.kind { + case testFileAbsent: + // File does not exist. + case testFileFile: + // Regular file. We use the token as the file contents. + if err := os.WriteFile(path, []byte(token), 0o666); err != nil { + t.Fatal(err) + } + case testFileDir: + // Directory. We create a subdir within the directory named "c_"+token. + // (The "c_" prefix is to distinguish this subdir from any files that may + // have the same name as the token.) + if err := os.Mkdir(path, 0o777); err != nil { + t.Fatal(err) + } + case testFileSymlink: + // Symlink. We create a symlink target named "s_"+base. + if runtime.GOOS == "plan9" { + t.Skip("symlinks not supported on " + runtime.GOOS) + } + linktarget := desc.target.ref.path(dir, "s_"+base) + if runtime.GOOS == "wasip1" && filepath.IsAbs(linktarget) { + t.Skip("absolute link targets not supported on " + runtime.GOOS) + } + fi = desc.target.create(t, dir, "s_"+base, token) + if err := os.Symlink(linktarget, path); err != nil { + t.Fatal(err) + } + default: + t.Fatalf("can't create file of kind: %v", desc.kind) + } + if desc.kind == testFileFile || desc.kind == testFileDir { + var err error + fi, err = os.Lstat(path) + if err != nil { + t.Fatal(err) + } + } + return fi +} + +// testRootDescribeFile returns a string identifying a file. +// +// It returns "" if f is nil. +// It returns "source" or "target" if f is the source or target file in the test. +// Otherwise, it returns "unknown file". +func (test *rootMultiTest) describeFile(t *testing.T, f *os.File) string { + if f == nil { + return "" + } + fi, err := f.Stat() + if err != nil { + t.Fatal(err) + } + switch { + case os.SameFile(fi, test.sourceInfo): + return "source" + case os.SameFile(fi, test.targetInfo): + return "target" + default: + return "unknown file" + } +} + +// dirTreeContents returns a description of the contents of directory. +// For example: +// +// drwxrwxrwx dir/ +// -rw-rw-rw- dir/file "file contents" +// Lrw-rw-rw- symlink => dir/file +func dirTreeContents(t *testing.T, dir string) (contents []string) { + root, err := os.OpenRoot(dir) + if err != nil { + t.Fatal(err) + } + defer root.Close() + fs.WalkDir(root.FS(), ".", func(path string, d fs.DirEntry, err error) error { + if path == "." { + return nil + } + info, err := d.Info() + if err != nil { + t.Fatal(err) + } + ent := info.Mode().String() + " " + path + switch d.Type() { + case fs.ModeDir: + ent += "/" + case fs.ModeSymlink: + target, err := root.Readlink(path) + if err != nil { + t.Fatal(err) + } + if filepath.IsAbs(target) { + relPath, err := filepath.Rel(dir, target) + if err == nil && filepath.IsLocal(relPath) { + target = "/.../" + relPath + } + } + ent += " => " + target + default: + f, err := root.Open(path) + if err != nil { + ent += " (unreadable)" + } else { + content, err := io.ReadAll(f) + if err != nil { + t.Fatal(err) + } + ent += fmt.Sprintf(" %q", content) + } + } + contents = append(contents, ent) + return nil + }) + return contents +} + +// TestRootMultiOpen tests os.Root.Open. +// +// This also serves as a prototypical example of using rootMultiTest +// (see also the doc comment on rootMultiTest above). +func TestRootMultiOpen(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + // This function will be run many times, with different inputs: + // - in and out of a Root + // - opening a file, directory, symlink, or nothing at all + // - opening various names: target, DIR/../target, /abs/path/to/target, etc. + // + // The test function should perform the requested operation + // (for example: open "target" in a Root), + // verify that the result is consistent with expectations, + // and then return a description of the result. + // + // The returned description is used to validate consistent behavior + // between operations in and out of a Root. + var open = os.Open + if test.root != nil { + open = test.root.Open + } + + test.setOp("Open(%q)", test.targetPath) // test's operation, for errors + f, gotErr := open(test.targetPath) + if gotErr == nil { + defer f.Close() + } + + // testRootDescribeFile returns a string identifying a file. + // + // This is always "source" or "target" for the source/target files in a test, + // or "" if f is nil. + // (Note that most tests use only a target file, no source.) + got := test.describeFile(t, f) + + switch { + case test.root != nil && test.target.escapes(): + // The operation escapes the root. + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileAbsent: + // The file does not exist ("absent"). + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + // The file name or a symlink target contain a trailing slash. + // Trailing slashes are handled differently on different platforms, + // so we won't try to assert an outcome when they are present. + // runRootMultiTest will verify that root.Open and os.Open + // produce consistent results. + default: + // We should have successfully opened the file. + test.wantError(t, gotErr, nil) + if want := "target"; got != want { + t.Fatalf("opened file %q, want %q", got, want) + } + } + + // Return the name of the file opened (possibly "" for nothing) and the error. + // runRootMultiTest will compare the results for in-a-root and out-of-a-root + // to validate that they are the same. + return got, gotErr + }) +} + +func TestRootMultiChmod(t *testing.T) { + if runtime.GOOS == "wasip1" { + t.Skip("Chmod not supported on " + runtime.GOOS) + } + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var ( + chmod = os.Chmod + stat = os.Stat + lstat = os.Lstat + ) + if test.root != nil { + chmod = test.root.Chmod + stat = test.root.Stat + lstat = test.root.Lstat + } + + // Using the wrong mode here can cause problems during test cleanup, + // if we leave a temp dir with a mode that prevents listing or removing + // its contents. + // + // read+execute permissions let us list directory contents, + // and we restore writability before deleting the temp dir. + wantMode := os.FileMode(0o500) // readable, executable + if runtime.GOOS == "windows" { + // On Windows, the only modes we support are the default (777/rwx) + // or read-only (444/r-x). Making a directory read-only doesn't prevent + // listing its contents, so we can use 444 here. + wantMode = 0o444 // readable + } + t.Cleanup(func() { + chmod(test.targetPath, 0o700) + }) + + test.setOp("Chmod(%q, %o)", test.targetPath, wantMode) + gotErr := chmod(test.targetPath, wantMode) + + escapes := test.target.escapes() + targetKind := test.target.finalKind() + if runtime.GOOS == "windows" { + // On Windows, Chmod("symlink") affects the link, not its target. + // See issue #71492. + stat = lstat + escapes = test.target.ref.escapes + targetKind = test.target.kind + } + + var gotMode fs.FileMode + switch { + case test.root != nil && escapes: + test.wantError(t, gotErr, os.ErrPathEscapes) + case targetKind == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + // Don't expect anything, just be consistent with the OS. + default: + test.wantError(t, gotErr, nil) + + fi, err := stat(test.targetPath) + if err != nil { + t.Fatalf("could not stat target: %v", err) + } + if runtime.GOOS == "windows" && !fi.Mode().IsRegular() { + // See issue #71492. + break + } + + gotMode = fi.Mode() & fs.ModePerm + if gotMode != wantMode { + t.Fatalf("file %q:\ngot mode: %v\nwant mode: %v", test.targetPath, gotMode, wantMode) + } + } + + if runtime.GOOS == "windows" && test.root == nil && gotErr != nil { + // On Windows, os.Chmod calls GetFileAttributes on the target. + // This seems to fail in a number of situations where the os.Root + // chmod path works. For now, just skip the consistency check + // when os.Chmod fails. + return "", errSkipRootConsistencyCheck + } + + return gotMode.String(), gotErr + }) +} + +func TestRootMultiCreate(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var create = os.Create + if test.root != nil { + create = test.root.Create + } + + test.setOp("Create(%q)", test.targetPath) // test's operation, for errors + f, gotErr := create(test.targetPath) + if gotErr == nil { + defer f.Close() + } + + switch { + case test.target.isError(): + test.wantError(t, gotErr, errAny) + case runtime.GOOS == "windows" && test.target.isSymlinkToDir(): + // The error here is because the link is a Windows directory link, + // not because the link target is a directory. + test.wantError(t, gotErr, errAny) + case test.root != nil && test.target.escapes(): + // The operation escapes the root. + test.wantError(t, gotErr, os.ErrPathEscapes) + default: + } + + return "", gotErr + }) +} + +func TestRootMultiLink(t *testing.T) { + if runtime.GOOS == "wasip1" { + switch os.Getenv("GOWASIRUNTIME") { + case "", "wasmtime": + // This test fails when run with wasmtime, because os.RemoveAll fails + // to remove the test tempdir. + t.Skip("test seems to tickle a wasmtime bug") + } + } + testenv.MustHaveLink(t) + runRootMultiTest2(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var ( + rename = os.Link + ) + if test.root != nil { + rename = test.root.Link + } + + test.setOp("Link(%q, %q)", test.sourcePath, test.targetPath) + gotErr := rename(test.sourcePath, test.targetPath) + + switch { + case test.root != nil && test.source.lescapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.source.lfinalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.source.kind == testFileSymlink: + // os.Link(old, new) may or may not deference old when it is a symlink. + // POSIX says that link(2) should deference the source, but implementations + // are inconsistent. + return "", errSkipRootConsistencyCheck + case test.source.slashSuffix() && test.source.lfinalKind() != testFileDir: + test.wantError(t, gotErr, errAny) + } + return "", gotErr + }) +} + +func TestRootMultiLstat(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var ( + lstat = os.Lstat + ) + if test.root != nil { + lstat = test.root.Lstat + } + + test.setOp("Lstat(%q)", test.targetPath) + gotStat, gotErr := lstat(test.targetPath) + + result := "" + if gotStat != nil { + result = gotStat.Mode().String() + } + + escapes := test.target.lescapes() + finalKind := test.target.lfinalKind() + if runtime.GOOS == "windows" && test.target.ref.hasSlashSuffix() { + // When the target of lstat has a trailing slash, + // Windows follows it. + escapes = test.target.escapes() + finalKind = test.target.finalKind() + } + + switch { + case test.root != nil && escapes: + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.kind == testFileAbsent: + // Target does not exist. + test.wantError(t, gotErr, errAny) + case finalKind == testFileSymlink: + test.wantError(t, gotErr, nil) + if got, want := gotStat.Mode().Type(), fs.ModeSymlink; got != want { + test.errorf(t, "got mode %v, want %v", got, want) + } + case gotErr != nil: + default: + if !os.SameFile(gotStat, test.targetInfo) { + test.errorf(t, "stat result is not for target file; want it to be") + } + } + + return result, gotErr + }) +} + +func TestRootMultiMkdir(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var ( + mkdir = os.Mkdir + stat = os.Stat + ) + if test.root != nil { + mkdir = test.root.Mkdir + stat = test.root.Stat + } + + test.setOp("Mkdir(%q, 0o777)", test.targetPath) + gotErr := mkdir(test.targetPath, 0o777) + + switch { + case test.root != nil && test.target.ref.escapes: + // "mkdir ../target", or equivalent escaping path. + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.slashSuffix() && test.target.kind == testFileSymlink: + // "mkdir symlink/", inconsistent behavior across platforms + // as to whether this follows the symlink or not. + // + // If the symlink escapes, this needs to be some kind of error though. + if test.root != nil && test.target.escapes() { + test.wantError(t, gotErr, errAny) + } + if runtime.GOOS == "openbsd" { + // Known inconsistency: OpenBSD doesn't resolve the final + // symlink when creating a directory. + return "", errSkipRootConsistencyCheck + } + case test.target.kind != testFileAbsent: + // "mkdir target", where target exists. + test.wantError(t, gotErr, errAny) + default: + test.wantError(t, gotErr, nil) + fi, err := stat(test.targetPath) + if err != nil { + t.Fatalf("could not stat target: %v", err) + } + if !fi.IsDir() { + t.Fatalf("%q: not a directory, expected it to be", test.targetPath) + } + } + return "", gotErr + }) +} + +func TestRootMultiRename(t *testing.T) { + if runtime.GOOS == "wasip1" { + switch os.Getenv("GOWASIRUNTIME") { + case "", "wasmtime": + // This test fails when run with wasmtime, because os.RemoveAll fails + // to remove the test tempdir. + t.Skip("test seems to tickle a wasmtime bug") + } + } + runRootMultiTest2(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var ( + rename = os.Rename + ) + if test.root != nil { + rename = test.root.Rename + } + + // TODO: target directory (if any) should be empty + + test.setOp("Rename(%q, %q)", test.sourcePath, test.targetPath) + gotErr := rename(test.sourcePath, test.targetPath) + + if runtime.GOOS == "windows" && + (test.source.finalKind() != test.target.finalKind() || test.source.kind == testFileSymlink || test.target.kind == testFileSymlink) { + // os.Rename on Windows is implemented using MoveFileEx, + // while Root.Rename is implemented using NtSetInformationFileEx + // with an explicit request for POSIX semantics. + // + // This means the two do not behave the same when renaming + // a file onto a directory or vice-versa. + // + // We should make this consistent, but for now just skip + // the consistency checks in this case. + return "", errSkipRootConsistencyCheck + } + + switch { + case test.root != nil && test.source.lescapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.source.lfinalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.source.slashSuffix() && test.source.lfinalKind() != testFileDir && runtime.GOOS != "js": + test.wantError(t, gotErr, errAny) + case test.root != nil && test.target.lescapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case runtime.GOOS == "plan9": + // Plan9 rename behaves differently. + // Just rely on consistency checks. + case test.target.lfinalKind() == testFileDir: + // POSIX rename() will replace an empty target directory, + // but os.Rename will not. + test.wantError(t, gotErr, errAny) + case test.source.lfinalKind() == testFileDir && test.target.lfinalKind() != testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.source.anySlashSuffix() || test.target.anySlashSuffix(): + if runtime.GOOS == "openbsd" { + // Known inconsistency: OpenBSD doesn't resolve the final + // symlink when creating a directory. + return "", errSkipRootConsistencyCheck + } + default: + test.wantError(t, gotErr, nil) + // TODO: check that the file is in its new location + } + + if runtime.GOOS == "linux" && (test.source.slashSuffix() || test.target.slashSuffix()) { + return "", errSkipRootConsistencyCheck + } + + return "", gotErr + }) +} + +func TestRootMultiReadFile(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var readFile = os.ReadFile + if test.root != nil { + readFile = test.root.ReadFile + } + + test.setOp("ReadFile(%q)", test.targetPath) + data, gotErr := readFile(test.targetPath) + var got string + if gotErr == nil { + got = string(data) + } + + switch { + case test.root != nil && test.target.escapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case runtime.GOOS == "plan9": + // Plan9 lets you read from directories. + // Just rely on consistency checks. + case test.target.finalKind() == testFileDir: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + // Trailing slashes are handled differently on different platforms, + // so we won't try to assert an outcome when they are present. + // runRootMultiTest will verify that root.ReadFile and os.ReadFile + // produce consistent results. + default: + test.wantError(t, gotErr, nil) + if want := "target"; got != want { + t.Fatalf("read file content %q, want %q", got, want) + } + } + + return got, gotErr + }) +} + +func TestRootMultiStat(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var stat = os.Stat + if test.root != nil { + stat = test.root.Stat + } + + test.setOp("Stat(%q)", test.targetPath) + gotStat, gotErr := stat(test.targetPath) + + switch { + case test.target.isError(): + test.wantError(t, gotErr, errAny) + case test.root != nil && test.target.escapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + default: + test.wantError(t, gotErr, nil) + if !os.SameFile(gotStat, test.targetInfo) { + test.errorf(t, "stat result is not for target file; want it to be") + } + } + return "", gotErr + }) +} + +func TestRootMultiRemove(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var remove = os.Remove + if test.root != nil { + remove = test.root.Remove + } + + test.setOp("Remove(%q)", test.targetPath) + gotErr := remove(test.targetPath) + + switch { + case test.root != nil && test.target.lescapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.kind == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + if runtime.GOOS == "linux" { + // Linux treats rmdir("symlink/") as an error when + // "symlink" is a symlink to a directory. + // Root.Remove prefers the POSIX interpretation + // of resolving the symlink. + return "", errSkipRootConsistencyCheck + } + default: + test.wantError(t, gotErr, nil) + } + return "", gotErr + }) +} + +func TestRootMultiRemoveAll(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var removeAll = os.RemoveAll + if test.root != nil { + removeAll = test.root.RemoveAll + } + + test.setOp("RemoveAll(%q)", test.targetPath) + gotErr := removeAll(test.targetPath) + + switch { + case test.root != nil && test.target.ref.escapes: + // This is only checking target.ref.escapes, + // not target.lescapes(), because RemoveAll strips + // terminal slashes. + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.anySlashSuffix(): + // We are inconsistent on some platforms on whether + // RemoveAll("symlink/") removes the link or the link target. + // Something worth addressing, but for now skip the check. + return "", errSkipRootConsistencyCheck + default: + test.wantError(t, gotErr, nil) + } + return "", gotErr + }) +} + +func TestRootMultiChtimes(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var chtimes = os.Chtimes + if test.root != nil { + chtimes = test.root.Chtimes + } + + now := time.Now() + test.setOp("Chtimes(%q, %v, %v)", test.targetPath, now, now) + gotErr := chtimes(test.targetPath, now, now) + + switch { + case test.target.isError(): + test.wantError(t, gotErr, errAny) + case test.root != nil && test.target.escapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + default: + test.wantError(t, gotErr, nil) + } + return "", gotErr + }) +} + +func TestRootMultiReadlink(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var readlink = os.Readlink + if test.root != nil { + readlink = test.root.Readlink + } + + test.setOp("Readlink(%q)", test.targetPath) + got, gotErr := readlink(test.targetPath) + if suffix, ok := strings.CutPrefix(got, test.dir); ok { + // Replace absolute path prefix with /.../ + got = "/..." + suffix + } + + switch { + case test.root != nil && test.target.lescapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.kind != testFileSymlink: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + default: + test.wantError(t, gotErr, nil) + } + return got, gotErr + }) +} + +func TestRootMultiWriteFile(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var writeFile = os.WriteFile + if test.root != nil { + writeFile = test.root.WriteFile + } + + test.setOp("WriteFile(%q, ...)", test.targetPath) + gotErr := writeFile(test.targetPath, []byte("data"), 0o666) + + switch { + case test.target.isError(): + test.wantError(t, gotErr, errAny) + case runtime.GOOS == "windows" && test.target.isSymlinkToDir(): + test.wantError(t, gotErr, errAny) + case test.root != nil && test.target.escapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileDir: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + default: + test.wantError(t, gotErr, nil) + } + return "", gotErr + }) +} + +func TestRootMultiOpenFile(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + var openFile = os.OpenFile + if test.root != nil { + openFile = test.root.OpenFile + } + + test.setOp("OpenFile(%q, O_RDONLY, 0)", test.targetPath) + f, gotErr := openFile(test.targetPath, os.O_RDONLY, 0) + if gotErr == nil { + defer f.Close() + } + + got := test.describeFile(t, f) + + switch { + case test.root != nil && test.target.escapes(): + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.target.finalKind() == testFileAbsent: + test.wantError(t, gotErr, errAny) + case test.target.anySlashSuffix(): + default: + test.wantError(t, gotErr, nil) + if want := "target"; got != want { + t.Fatalf("opened file %q, want %q", got, want) + } + } + + return got, gotErr + }) +} diff --git a/src/os/root_unix.go b/src/os/root_unix.go index 885a8353ebc455..e88058715387b3 100644 --- a/src/os/root_unix.go +++ b/src/os/root_unix.go @@ -62,7 +62,7 @@ func newRoot(fd int, name string) (*Root, error) { // openRootInRoot is Root.OpenRoot. func openRootInRoot(r *Root, name string) (*Root, error) { - fd, err := doInRoot(r, name, nil, func(parent int, name string) (fd int, err error) { + fd, err := doInRoot(r, name, 0, nil, func(parent int, name string, endsInSlash bool) (fd int, err error) { ignoringEINTR(func() error { fd, err = unix.Openat(parent, name, syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) if isNoFollowErr(err) { @@ -80,9 +80,10 @@ func openRootInRoot(r *Root, name string) (*Root, error) { // rootOpenFileNolog is Root.OpenFile. func rootOpenFileNolog(root *Root, name string, flag int, perm FileMode) (*File, error) { - fd, err := doInRoot(root, name, nil, func(parent int, name string) (fd int, err error) { + fd, err := doInRoot(root, name, 0, nil, func(parent int, name string, endsInSlash bool) (fd int, err error) { ignoringEINTR(func() error { - fd, err = unix.Openat(parent, name, syscall.O_NOFOLLOW|syscall.O_CLOEXEC|flag, uint32(perm)) + openFlag := syscall.O_NOFOLLOW | syscall.O_CLOEXEC | flag + fd, err = unix.Openat(parent, name, openFlag, uint32(perm)) if err != nil { // Never follow symlinks when O_CREATE|O_EXCL, no matter // what error the OS returns. @@ -129,16 +130,15 @@ func rootOpenDir(parent int, name string) (int, error) { } func rootStat(r *Root, name string, lstat bool) (FileInfo, error) { - fi, err := doInRoot(r, name, nil, func(parent sysfdType, n string) (FileInfo, error) { - var fs fileStat - if err := unix.Fstatat(parent, n, &fs.sys, unix.AT_SYMLINK_NOFOLLOW); err != nil { + fi, err := doInRoot(r, name, 0, nil, func(parent sysfdType, n string, endsInSlash bool) (FileInfo, error) { + fi, err := lstatatWithName(parent, name, n) + if err != nil { return nil, err } - fillFileStatFromSys(&fs, name) - if !lstat && fs.Mode()&ModeSymlink != 0 { + if !lstat && fi.Mode()&ModeSymlink != 0 { return nil, checkSymlink(parent, n, syscall.ELOOP) } - return &fs, nil + return fi, nil }) if err != nil { return nil, &PathError{Op: "statat", Path: name, Err: err} @@ -147,7 +147,7 @@ func rootStat(r *Root, name string, lstat bool) (FileInfo, error) { } func rootSymlink(r *Root, oldname, newname string) error { - _, err := doInRoot(r, newname, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, newname, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, symlinkat(oldname, parent, name) }) if err != nil { @@ -257,13 +257,17 @@ func symlinkat(oldname string, newfd int, newname string) error { return unix.Symlinkat(oldname, newfd, newname) } -func modeAt(parent int, name string) (FileMode, error) { +func lstatat(parent int, name string) (FileInfo, error) { + return lstatatWithName(parent, name, name) +} + +func lstatatWithName(parent int, origName, name string) (FileInfo, error) { var fs fileStat if err := unix.Fstatat(parent, name, &fs.sys, unix.AT_SYMLINK_NOFOLLOW); err != nil { - return 0, err + return nil, err } - fillFileStatFromSys(&fs, name) - return fs.mode, nil + fillFileStatFromSys(&fs, origName) + return &fs, nil } // checkSymlink resolves the symlink name in parent, @@ -301,3 +305,10 @@ func readlinkat(fd int, name string) (string, error) { } } } + +// isDirectoryLink always returns false, because Unix systems don't have separate +// symlink types for files and directories. +// (See the Windows version of this function for more details.) +func isDirectoryLink(fi FileInfo) bool { + return false +} diff --git a/src/os/root_unix_test.go b/src/os/root_unix_test.go index b4b37c2be9dea2..9790a13164caf7 100644 --- a/src/os/root_unix_test.go +++ b/src/os/root_unix_test.go @@ -71,9 +71,8 @@ func TestRootLchown(t *testing.T) { groups = append(groups, os.Getgid()) for _, test := range rootTestCases { test.run(t, func(t *testing.T, target string, root *os.Root) { - wantError := test.wantError if test.ltarget != "" { - wantError = false + test.wantError = false target = filepath.Join(root.Name(), test.ltarget) } else if target != "" { if err := os.WriteFile(target, nil, 0o666); err != nil { @@ -82,7 +81,7 @@ func TestRootLchown(t *testing.T) { } for _, gid := range groups { err := root.Lchown(test.open, -1, gid) - if errEndsTest(t, err, wantError, "root.Lchown(%q, -1, %v)", test.open, gid) { + if errEndsTest(t, err, test.wantError, "root.Lchown(%q, -1, %v)", test.open, gid) { return } checkUidGid(t, target, int(sys.Uid), gid) diff --git a/src/os/root_windows.go b/src/os/root_windows.go index da995e6d7f5c3b..6bd37c50a1fc5b 100644 --- a/src/os/root_windows.go +++ b/src/os/root_windows.go @@ -119,7 +119,9 @@ func newRoot(fd syscall.Handle, name string) (*Root, error) { // openRootInRoot is Root.OpenRoot. func openRootInRoot(r *Root, name string) (*Root, error) { - fd, err := doInRoot(r, name, nil, rootOpenDir) + fd, err := doInRoot(r, name, 0, nil, func(parent syscall.Handle, name string, endsInSlash bool) (syscall.Handle, error) { + return rootOpenDir(parent, name) + }) if err != nil { return nil, &PathError{Op: "openat", Path: name, Err: err} } @@ -128,7 +130,10 @@ func openRootInRoot(r *Root, name string) (*Root, error) { // rootOpenFileNolog is Root.OpenFile. func rootOpenFileNolog(root *Root, name string, flag int, perm FileMode) (*File, error) { - fd, err := doInRoot(root, name, nil, func(parent syscall.Handle, name string) (syscall.Handle, error) { + fd, err := doInRoot(root, name, doInRootNoHandleTerminalSlash, nil, func(parent syscall.Handle, name string, endsInSlash bool) (syscall.Handle, error) { + if endsInSlash { + flag |= windows.O_DIRECTORY + } return openat(parent, name, uint64(flag), perm) }) if err != nil { @@ -204,15 +209,16 @@ func rootOpenDir(parent syscall.Handle, name string) (syscall.Handle, error) { } func rootStat(r *Root, name string, lstat bool) (FileInfo, error) { - if len(name) > 0 && IsPathSeparator(name[len(name)-1]) { - // When a filename ends with a path separator, - // Lstat behaves like Stat. + var flags uint + if lstat { + // Follow symlinks in the last path component when the path + // ends with a path separator. // - // This behavior is not based on a principled decision here, - // merely the empirical evidence that Lstat behaves this way. - lstat = false + // This is not the usual behavior for Windows path resolution, + // but empirically os.Lstat behaves this way. + flags = doInRootAlwaysResolveTerminalSlash } - fi, err := doInRoot(r, name, nil, func(parent syscall.Handle, n string) (FileInfo, error) { + fi, err := doInRoot(r, name, flags, nil, func(parent syscall.Handle, n string, endsInSlash bool) (FileInfo, error) { fd, err := openat(parent, n, windows.O_FILE_FLAG_OPEN_REPARSE_POINT, 0) if err != nil { return nil, err @@ -274,7 +280,7 @@ func rootSymlink(r *Root, oldname, newname string) error { flags |= windows.SYMLINKAT_RELATIVE } - _, err := doInRoot(r, newname, nil, func(parent sysfdType, name string) (struct{}, error) { + _, err := doInRoot(r, newname, 0, nil, func(parent sysfdType, name string, endsInSlash bool) (struct{}, error) { return struct{}{}, windows.Symlinkat(oldname, parent, name, flags) }) if err != nil { @@ -381,6 +387,18 @@ func linkat(oldfd syscall.Handle, oldname string, newfd syscall.Handle, newname return windows.Linkat(oldfd, oldname, newfd, newname) } +// checkSymlink resolves the symlink name in parent, +// and returns errSymlink with the link contents. +// +// If name is not a symlink, return origError. +func checkSymlink(parent syscall.Handle, name string, origError error) error { + link, err := readlinkat(parent, name) + if err != nil { + return origError + } + return errSymlink(link) +} + func readlinkat(dirfd syscall.Handle, name string) (string, error) { fd, err := openat(dirfd, name, windows.O_FILE_FLAG_OPEN_REPARSE_POINT, 0) if err != nil { @@ -390,15 +408,23 @@ func readlinkat(dirfd syscall.Handle, name string) (string, error) { return readReparseLinkHandle(fd) } -func modeAt(parent syscall.Handle, name string) (FileMode, error) { - fd, err := openat(parent, name, windows.O_FILE_FLAG_OPEN_REPARSE_POINT|windows.O_DIRECTORY, 0) +func lstatat(parent syscall.Handle, name string) (FileInfo, error) { + fd, err := openat(parent, name, windows.O_FILE_FLAG_OPEN_REPARSE_POINT, 0) if err != nil { - return 0, err + return nil, err } defer syscall.CloseHandle(fd) fi, err := statHandle(name, fd) if err != nil { - return 0, err + return nil, err } - return fi.Mode(), nil + return fi, nil +} + +// isDirectoryLink reports whether fi (assumed to be a symlink) is a directory link. +// Windows symlinks come in two flavors: file and directory. This function distinguishes +// between the two. +func isDirectoryLink(fi FileInfo) bool { + fs, ok := fi.(*fileStat) + return ok && fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 } From c19862e5f8415b4f24b189d065ed739517c548ba Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Tue, 7 Jul 2026 12:21:32 -0700 Subject: [PATCH 12/12] [release-branch.go1.26] go1.26.5 Change-Id: I87dc3d84cde11db83a0d88a60262a38fc429838d Reviewed-on: https://go-review.googlesource.com/c/go/+/797740 Auto-Submit: Gopher Robot Reviewed-by: David Chase Reviewed-by: Junyang Shao TryBot-Bypass: Gopher Robot --- VERSION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 5da5a8e2892581..9b642ac23a6b1e 100644 --- a/VERSION +++ b/VERSION @@ -1,2 +1,2 @@ -go1.26.4 -time 2026-05-29T15:26:39Z +go1.26.5 +time 2026-07-01T21:24:27Z