[release-branch.go1.22] os: RemoveAll: fix symlink race for unix
Since all the platforms now support O_DIRECTORY flag for open, it can be
used to (together with O_NOFOLLOW) to ensure we open a directory, thus
eliminating the need to call stat before open. This fixes the symlink race,
when a directory is replaced by a symlink in between stat and open calls.
While at it, rename openFdAt to openDirAt, because this function is (and was)
meant for directories only.
NOTE Solaris supports O_DIRECTORY since before Solaris 11 (which is the
only version Go supports since supported version now), and Illumos
always had it. The only missing piece was O_DIRECTORY flag value, which
is taken from golang.org/x/sys/unix.
Fixes #67696.
Change-Id: Ic1111d688eebc8804a87d39d3261c2a6eb33f176
Reviewed-on: https://go-review.googlesource.com/c/go/+/589056
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/src/os/file_unix.go b/src/os/file_unix.go
index a527b23..649ec3e 100644
--- a/src/os/file_unix.go
+++ b/src/os/file_unix.go
@@ -157,7 +157,7 @@
kindNonBlock
// kindNoPoll means that we should not put the descriptor into
// non-blocking mode, because we know it is not a pipe or FIFO.
- // Used by openFdAt for directories.
+ // Used by openDirAt for directories.
kindNoPoll
)
@@ -256,7 +256,7 @@
const DevNull = "/dev/null"
// openFileNolog is the Unix implementation of OpenFile.
-// Changes here should be reflected in openFdAt, if relevant.
+// Changes here should be reflected in openDirAt, if relevant.
func openFileNolog(name string, flag int, perm FileMode) (*File, error) {
setSticky := false
if !supportsCreateWithStickyBit && flag&O_CREATE != 0 && perm&ModeSticky != 0 {
diff --git a/src/os/removeall_at.go b/src/os/removeall_at.go
index 8ea5df4..774ca15 100644
--- a/src/os/removeall_at.go
+++ b/src/os/removeall_at.go
@@ -74,22 +74,7 @@
if err != syscall.EISDIR && err != syscall.EPERM && err != syscall.EACCES {
return &PathError{Op: "unlinkat", Path: base, Err: err}
}
-
- // Is this a directory we need to recurse into?
- var statInfo syscall.Stat_t
- statErr := ignoringEINTR(func() error {
- return unix.Fstatat(parentFd, base, &statInfo, unix.AT_SYMLINK_NOFOLLOW)
- })
- if statErr != nil {
- if IsNotExist(statErr) {
- return nil
- }
- return &PathError{Op: "fstatat", Path: base, Err: statErr}
- }
- if statInfo.Mode&syscall.S_IFMT != syscall.S_IFDIR {
- // Not a directory; return the error from the unix.Unlinkat.
- return &PathError{Op: "unlinkat", Path: base, Err: err}
- }
+ uErr := err
// Remove the directory's entries.
var recurseErr error
@@ -98,11 +83,15 @@
var respSize int
// Open the directory to recurse into
- file, err := openFdAt(parentFd, base)
+ file, err := openDirAt(parentFd, base)
if err != nil {
if IsNotExist(err) {
return nil
}
+ if err == syscall.ENOTDIR {
+ // Not a directory; return the error from the unix.Unlinkat.
+ return &PathError{Op: "unlinkat", Path: base, Err: uErr}
+ }
recurseErr = &PathError{Op: "openfdat", Path: base, Err: err}
break
}
@@ -168,16 +157,19 @@
return &PathError{Op: "unlinkat", Path: base, Err: unlinkError}
}
-// openFdAt opens path relative to the directory in fd.
-// Other than that this should act like openFileNolog.
+// openDirAt opens a directory name relative to the directory referred to by
+// the file descriptor dirfd. If name is anything but a directory (this
+// includes a symlink to one), it should return an error. Other than that this
+// should act like openFileNolog.
+//
// This acts like openFileNolog rather than OpenFile because
// we are going to (try to) remove the file.
// The contents of this file are not relevant for test caching.
-func openFdAt(dirfd int, name string) (*File, error) {
+func openDirAt(dirfd int, name string) (*File, error) {
var r int
for {
var e error
- r, e = unix.Openat(dirfd, name, O_RDONLY|syscall.O_CLOEXEC, 0)
+ r, e = unix.Openat(dirfd, name, O_RDONLY|syscall.O_CLOEXEC|syscall.O_DIRECTORY|syscall.O_NOFOLLOW, 0)
if e == nil {
break
}
diff --git a/src/syscall/zerrors_solaris_amd64.go b/src/syscall/zerrors_solaris_amd64.go
index 4a1d9c3..b2c81d9 100644
--- a/src/syscall/zerrors_solaris_amd64.go
+++ b/src/syscall/zerrors_solaris_amd64.go
@@ -634,6 +634,7 @@
O_APPEND = 0x8
O_CLOEXEC = 0x800000
O_CREAT = 0x100
+ O_DIRECTORY = 0x1000000
O_DSYNC = 0x40
O_EXCL = 0x400
O_EXEC = 0x400000
