Skip to content

Commit 6d41809

Browse files
neildgopherbot
authored andcommitted
os: avoid symlink races in RemoveAll on Windows
Make the openat-using version of RemoveAll use the appropriate Windows equivalent, via new portable (but internal) functions added for os.Root. We could reimplement everything in terms of os.Root, but this is a bit simpler and keeps the existing code structure. Fixes #52745 Change-Id: I0eba0286398b351f2ee9abaa60e1675173988787 Reviewed-on: https://go-review.googlesource.com/c/go/+/661575 Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Damien Neil <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent c6a1dc4 commit 6d41809

10 files changed

+83
-28
lines changed

src/internal/syscall/unix/constants.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build unix
5+
//go:build unix || wasip1
66

77
package unix
88

src/internal/syscall/unix/nofollow_posix.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build unix && !dragonfly && !freebsd && !netbsd
5+
//go:build (unix && !dragonfly && !freebsd && !netbsd) || wasip1
66

77
package unix
88

src/internal/syscall/windows/at_windows.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func Mkdirat(dirfd syscall.Handle, name string, mode uint32) error {
188188
return nil
189189
}
190190

191-
func Deleteat(dirfd syscall.Handle, name string) error {
191+
func Deleteat(dirfd syscall.Handle, name string, options uint32) error {
192192
objAttrs := &OBJECT_ATTRIBUTES{}
193193
if err := objAttrs.init(dirfd, name); err != nil {
194194
return err
@@ -200,7 +200,7 @@ func Deleteat(dirfd syscall.Handle, name string) error {
200200
objAttrs,
201201
&IO_STATUS_BLOCK{},
202202
FILE_SHARE_DELETE|FILE_SHARE_READ|FILE_SHARE_WRITE,
203-
FILE_OPEN_REPARSE_POINT|FILE_OPEN_FOR_BACKUP_INTENT|FILE_SYNCHRONOUS_IO_NONALERT,
203+
FILE_OPEN_REPARSE_POINT|FILE_OPEN_FOR_BACKUP_INTENT|FILE_SYNCHRONOUS_IO_NONALERT|options,
204204
)
205205
if err != nil {
206206
return ntCreateFileError(err, 0)

src/os/path_windows.go

+10
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ func IsPathSeparator(c uint8) bool {
2121
return c == '\\' || c == '/'
2222
}
2323

24+
// splitPath returns the base name and parent directory.
25+
func splitPath(path string) (string, string) {
26+
dirname, basename := filepathlite.Split(path)
27+
volnamelen := filepathlite.VolumeNameLen(dirname)
28+
for len(dirname) > volnamelen && IsPathSeparator(dirname[len(dirname)-1]) {
29+
dirname = dirname[:len(dirname)-1]
30+
}
31+
return dirname, basename
32+
}
33+
2434
func dirname(path string) string {
2535
vol := filepathlite.VolumeName(path)
2636
i := len(path) - 1

src/os/removeall_at.go

+10-22
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build unix
5+
//go:build unix || wasip1 || windows
66

77
package os
88

99
import (
10-
"internal/syscall/unix"
1110
"io"
1211
"syscall"
1312
)
@@ -56,11 +55,10 @@ func removeAll(path string) error {
5655
}
5756

5857
func removeAllFrom(parent *File, base string) error {
59-
parentFd := int(parent.Fd())
58+
parentFd := sysfdType(parent.Fd())
59+
6060
// Simple case: if Unlink (aka remove) works, we're done.
61-
err := ignoringEINTR(func() error {
62-
return unix.Unlinkat(parentFd, base, 0)
63-
})
61+
err := removefileat(parentFd, base)
6462
if err == nil || IsNotExist(err) {
6563
return nil
6664
}
@@ -82,13 +80,13 @@ func removeAllFrom(parent *File, base string) error {
8280
const reqSize = 1024
8381
var respSize int
8482

85-
// Open the directory to recurse into
83+
// Open the directory to recurse into.
8684
file, err := openDirAt(parentFd, base)
8785
if err != nil {
8886
if IsNotExist(err) {
8987
return nil
9088
}
91-
if err == syscall.ENOTDIR || err == unix.NoFollowErrno {
89+
if err == syscall.ENOTDIR || isErrNoFollow(err) {
9290
// Not a directory; return the error from the unix.Unlinkat.
9391
return &PathError{Op: "unlinkat", Path: base, Err: uErr}
9492
}
@@ -144,9 +142,7 @@ func removeAllFrom(parent *File, base string) error {
144142
}
145143

146144
// Remove the directory itself.
147-
unlinkError := ignoringEINTR(func() error {
148-
return unix.Unlinkat(parentFd, base, unix.AT_REMOVEDIR)
149-
})
145+
unlinkError := removedirat(parentFd, base)
150146
if unlinkError == nil || IsNotExist(unlinkError) {
151147
return nil
152148
}
@@ -165,18 +161,10 @@ func removeAllFrom(parent *File, base string) error {
165161
// This acts like openFileNolog rather than OpenFile because
166162
// we are going to (try to) remove the file.
167163
// The contents of this file are not relevant for test caching.
168-
func openDirAt(dirfd int, name string) (*File, error) {
169-
r, err := ignoringEINTR2(func() (int, error) {
170-
return unix.Openat(dirfd, name, O_RDONLY|syscall.O_CLOEXEC|syscall.O_DIRECTORY|syscall.O_NOFOLLOW, 0)
171-
})
164+
func openDirAt(dirfd sysfdType, name string) (*File, error) {
165+
fd, err := rootOpenDir(dirfd, name)
172166
if err != nil {
173167
return nil, err
174168
}
175-
176-
if !supportsCloseOnExec {
177-
syscall.CloseOnExec(r)
178-
}
179-
180-
// We use kindNoPoll because we know that this is a directory.
181-
return newFile(r, name, kindNoPoll, false), nil
169+
return newDirFile(fd, name)
182170
}

src/os/removeall_noat.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build !unix
5+
//go:build (js && wasm) || plan9
66

77
package os
88

src/os/removeall_unix.go

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build unix || wasip1
6+
7+
package os
8+
9+
import (
10+
"internal/syscall/unix"
11+
)
12+
13+
func isErrNoFollow(err error) bool {
14+
return err == unix.NoFollowErrno
15+
}
16+
17+
func newDirFile(fd int, name string) (*File, error) {
18+
// We use kindNoPoll because we know that this is a directory.
19+
return newFile(fd, name, kindNoPoll, false), nil
20+
}

src/os/removeall_windows.go

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build windows
6+
7+
package os
8+
9+
import "syscall"
10+
11+
func isErrNoFollow(err error) bool {
12+
return err == syscall.ELOOP
13+
}
14+
15+
func newDirFile(fd syscall.Handle, name string) (*File, error) {
16+
return newFile(fd, name, "file"), nil
17+
}

src/os/root_unix.go

+12
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,18 @@ func removeat(fd int, name string) error {
219219
return e
220220
}
221221

222+
func removefileat(fd int, name string) error {
223+
return ignoringEINTR(func() error {
224+
return unix.Unlinkat(fd, name, 0)
225+
})
226+
}
227+
228+
func removedirat(fd int, name string) error {
229+
return ignoringEINTR(func() error {
230+
return unix.Unlinkat(fd, name, unix.AT_REMOVEDIR)
231+
})
232+
}
233+
222234
func renameat(oldfd int, oldname string, newfd int, newname string) error {
223235
return unix.Renameat(oldfd, oldname, newfd, newname)
224236
}

src/os/root_windows.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,15 @@ func mkdirat(dirfd syscall.Handle, name string, perm FileMode) error {
336336
}
337337

338338
func removeat(dirfd syscall.Handle, name string) error {
339-
return windows.Deleteat(dirfd, name)
339+
return windows.Deleteat(dirfd, name, 0)
340+
}
341+
342+
func removefileat(dirfd syscall.Handle, name string) error {
343+
return windows.Deleteat(dirfd, name, windows.FILE_NON_DIRECTORY_FILE)
344+
}
345+
346+
func removedirat(dirfd syscall.Handle, name string) error {
347+
return windows.Deleteat(dirfd, name, windows.FILE_DIRECTORY_FILE)
340348
}
341349

342350
func chtimesat(dirfd syscall.Handle, name string, atime time.Time, mtime time.Time) error {

0 commit comments

Comments
 (0)