]> Cypherpunks repositories - gostls13.git/commitdiff
os: add Root.Remove
authorDamien Neil <dneil@google.com>
Tue, 12 Nov 2024 16:16:10 +0000 (17:16 +0100)
committerDamien Neil <dneil@google.com>
Wed, 20 Nov 2024 23:21:14 +0000 (23:21 +0000)
For #67002

Change-Id: Ibbf44c0bf62f53695a7399ba0dae5b84d5efd374
Reviewed-on: https://go-review.googlesource.com/c/go/+/627076
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

16 files changed:
api/next/67002.txt
src/internal/syscall/unix/at_wasip1.go
src/internal/syscall/windows/at_windows.go
src/internal/syscall/windows/syscall_windows.go
src/internal/syscall/windows/types_windows.go
src/internal/syscall/windows/zsyscall_windows.go
src/os/os_test.go
src/os/root.go
src/os/root_js.go
src/os/root_noopenat.go
src/os/root_openat.go
src/os/root_plan9.go
src/os/root_test.go
src/os/root_unix.go
src/os/root_windows.go
src/os/root_windows_test.go

index 861ffe95165afd62ddc3be09df454ffe6842f32b..00248e1070597a9232de7b24783a155f594fa561 100644 (file)
@@ -6,4 +6,5 @@ pkg os, method (*Root) Name() string #67002
 pkg os, method (*Root) Open(string) (*File, error) #67002
 pkg os, method (*Root) OpenFile(string, int, fs.FileMode) (*File, error) #67002
 pkg os, method (*Root) OpenRoot(string) (*Root, error) #67002
+pkg os, method (*Root) Remove(string) error #67002
 pkg os, type Root struct #67002
index 45ae22afccd67e5b77fde1a99d3e866a3030cf84..2be7ef363004104e76897ba7e3bc9872f5a3e533 100644 (file)
@@ -17,8 +17,34 @@ const (
        // to avoid changing AccessTime or ModifiedTime.
        // Its value must match syscall/fs_wasip1.go
        UTIME_OMIT = -0x2
+
+       AT_REMOVEDIR = 0x200
 )
 
+func Unlinkat(dirfd int, path string, flags int) error {
+       if flags&AT_REMOVEDIR == 0 {
+               return errnoErr(path_unlink_file(
+                       int32(dirfd),
+                       unsafe.StringData(path),
+                       size(len(path)),
+               ))
+       } else {
+               return errnoErr(path_remove_directory(
+                       int32(dirfd),
+                       unsafe.StringData(path),
+                       size(len(path)),
+               ))
+       }
+}
+
+//go:wasmimport wasi_snapshot_preview1 path_unlink_file
+//go:noescape
+func path_unlink_file(fd int32, path *byte, pathLen size) syscall.Errno
+
+//go:wasmimport wasi_snapshot_preview1 path_remove_directory
+//go:noescape
+func path_remove_directory(fd int32, path *byte, pathLen size) syscall.Errno
+
 func Openat(dirfd int, path string, flags int, perm uint32) (int, error) {
        return syscall.Openat(dirfd, path, flags, perm)
 }
index af8167dd06d4542bf1d8ffe2f6b3bc7006ed01a7..72780139a066243702e6773df01ef51872458911 100644 (file)
@@ -6,6 +6,7 @@ package windows
 
 import (
        "syscall"
+       "unsafe"
 )
 
 // Openat flags not supported by syscall.Open.
@@ -171,3 +172,72 @@ func Mkdirat(dirfd syscall.Handle, name string, mode uint32) error {
        syscall.CloseHandle(h)
        return nil
 }
+
+func Deleteat(dirfd syscall.Handle, name string) error {
+       objAttrs := &OBJECT_ATTRIBUTES{}
+       if err := objAttrs.init(dirfd, name); err != nil {
+               return err
+       }
+       var h syscall.Handle
+       err := NtOpenFile(
+               &h,
+               DELETE,
+               objAttrs,
+               &IO_STATUS_BLOCK{},
+               FILE_SHARE_DELETE|FILE_SHARE_READ|FILE_SHARE_WRITE,
+               FILE_OPEN_REPARSE_POINT|FILE_OPEN_FOR_BACKUP_INTENT,
+       )
+       if err != nil {
+               return ntCreateFileError(err, 0)
+       }
+       defer syscall.CloseHandle(h)
+
+       const (
+               FileDispositionInformation   = 13
+               FileDispositionInformationEx = 64
+       )
+
+       // First, attempt to delete the file using POSIX semantics
+       // (which permit a file to be deleted while it is still open).
+       // This matches the behavior of DeleteFileW.
+       err = NtSetInformationFile(
+               h,
+               &IO_STATUS_BLOCK{},
+               uintptr(unsafe.Pointer(&FILE_DISPOSITION_INFORMATION_EX{
+                       Flags: FILE_DISPOSITION_DELETE |
+                               FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK |
+                               FILE_DISPOSITION_POSIX_SEMANTICS |
+                               // This differs from DeleteFileW, but matches os.Remove's
+                               // behavior on Unix platforms of permitting deletion of
+                               // read-only files.
+                               FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE,
+               })),
+               uint32(unsafe.Sizeof(FILE_DISPOSITION_INFORMATION_EX{})),
+               FileDispositionInformationEx,
+       )
+       switch err {
+       case nil:
+               return nil
+       case STATUS_CANNOT_DELETE, STATUS_DIRECTORY_NOT_EMPTY:
+               return err.(NTStatus).Errno()
+       }
+
+       // If the prior deletion failed, the filesystem either doesn't support
+       // POSIX semantics (for example, FAT), or hasn't implemented
+       // FILE_DISPOSITION_INFORMATION_EX.
+       //
+       // Try again.
+       err = NtSetInformationFile(
+               h,
+               &IO_STATUS_BLOCK{},
+               uintptr(unsafe.Pointer(&FILE_DISPOSITION_INFORMATION{
+                       DeleteFile: true,
+               })),
+               uint32(unsafe.Sizeof(FILE_DISPOSITION_INFORMATION{})),
+               FileDispositionInformation,
+       )
+       if st, ok := err.(NTStatus); ok {
+               return st.Errno()
+       }
+       return err
+}
index 924f4951e71e59d213fa4384d8267ddea20d7d7f..f6fbf199bfb0ad39d17abc277ce76ae337c32369 100644 (file)
@@ -523,10 +523,14 @@ func (s NTStatus) Error() string {
 // If this list starts getting long, we should consider generating the full set.
 const (
        STATUS_FILE_IS_A_DIRECTORY       NTStatus = 0xC00000BA
+       STATUS_DIRECTORY_NOT_EMPTY       NTStatus = 0xC0000101
        STATUS_NOT_A_DIRECTORY           NTStatus = 0xC0000103
+       STATUS_CANNOT_DELETE             NTStatus = 0xC0000121
        STATUS_REPARSE_POINT_ENCOUNTERED NTStatus = 0xC000050B
 )
 
 // NT Native APIs
 //sys   NtCreateFile(handle *syscall.Handle, access uint32, oa *OBJECT_ATTRIBUTES, iosb *IO_STATUS_BLOCK, allocationSize *int64, attributes uint32, share uint32, disposition uint32, options uint32, eabuffer uintptr, ealength uint32) (ntstatus error) = ntdll.NtCreateFile
+//sys   NtOpenFile(handle *syscall.Handle, access uint32, oa *OBJECT_ATTRIBUTES, iosb *IO_STATUS_BLOCK, share uint32, options uint32) (ntstatus error) = ntdll.NtOpenFile
 //sys   rtlNtStatusToDosErrorNoTeb(ntstatus NTStatus) (ret syscall.Errno) = ntdll.RtlNtStatusToDosErrorNoTeb
+//sys   NtSetInformationFile(handle syscall.Handle, iosb *IO_STATUS_BLOCK, inBuffer uintptr, inBufferLen uint32, class uint32) (ntstatus error) = ntdll.NtSetInformationFile
index 514feafae44dcf82dfbac4aebb03ec5abd7352f3..6ae37afff8bc2c3b33865216adf2429d98ac07ab 100644 (file)
@@ -196,3 +196,23 @@ const (
        FILE_OPEN_NO_RECALL            = 0x00400000
        FILE_OPEN_FOR_FREE_SPACE_QUERY = 0x00800000
 )
+
+// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information
+type FILE_DISPOSITION_INFORMATION struct {
+       DeleteFile bool
+}
+
+// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex
+type FILE_DISPOSITION_INFORMATION_EX struct {
+       Flags uint32
+}
+
+// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex
+const (
+       FILE_DISPOSITION_DO_NOT_DELETE             = 0x00000000
+       FILE_DISPOSITION_DELETE                    = 0x00000001
+       FILE_DISPOSITION_POSIX_SEMANTICS           = 0x00000002
+       FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK = 0x00000004
+       FILE_DISPOSITION_ON_CLOSE                  = 0x00000008
+       FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE = 0x00000010
+)
index c81bc399ff76705a005d52471fabf0a2f9580b2c..6a6ea7bdc06ba8f8b04abbebd6a0b70cca5062d4 100644 (file)
@@ -93,6 +93,8 @@ var (
        procNetUserDel                        = modnetapi32.NewProc("NetUserDel")
        procNetUserGetLocalGroups             = modnetapi32.NewProc("NetUserGetLocalGroups")
        procNtCreateFile                      = modntdll.NewProc("NtCreateFile")
+       procNtOpenFile                        = modntdll.NewProc("NtOpenFile")
+       procNtSetInformationFile              = modntdll.NewProc("NtSetInformationFile")
        procRtlGetVersion                     = modntdll.NewProc("RtlGetVersion")
        procRtlNtStatusToDosErrorNoTeb        = modntdll.NewProc("RtlNtStatusToDosErrorNoTeb")
        procGetProcessMemoryInfo              = modpsapi.NewProc("GetProcessMemoryInfo")
@@ -477,6 +479,22 @@ func NtCreateFile(handle *syscall.Handle, access uint32, oa *OBJECT_ATTRIBUTES,
        return
 }
 
+func NtOpenFile(handle *syscall.Handle, access uint32, oa *OBJECT_ATTRIBUTES, iosb *IO_STATUS_BLOCK, share uint32, options uint32) (ntstatus error) {
+       r0, _, _ := syscall.Syscall6(procNtOpenFile.Addr(), 6, uintptr(unsafe.Pointer(handle)), uintptr(access), uintptr(unsafe.Pointer(oa)), uintptr(unsafe.Pointer(iosb)), uintptr(share), uintptr(options))
+       if r0 != 0 {
+               ntstatus = NTStatus(r0)
+       }
+       return
+}
+
+func NtSetInformationFile(handle syscall.Handle, iosb *IO_STATUS_BLOCK, inBuffer uintptr, inBufferLen uint32, class uint32) (ntstatus error) {
+       r0, _, _ := syscall.Syscall6(procNtSetInformationFile.Addr(), 5, uintptr(handle), uintptr(unsafe.Pointer(iosb)), uintptr(inBuffer), uintptr(inBufferLen), uintptr(class), 0)
+       if r0 != 0 {
+               ntstatus = NTStatus(r0)
+       }
+       return
+}
+
 func rtlGetVersion(info *_OSVERSIONINFOW) {
        syscall.Syscall(procRtlGetVersion.Addr(), 1, uintptr(unsafe.Pointer(info)), 0, 0)
        return
index e891c1a422acd3a4dda4191d583e6e7e7c6ee938..c646ca82463f231710c48fe440d8f717945c5023 100644 (file)
@@ -3815,3 +3815,23 @@ func TestAppendDoesntOverwrite(t *testing.T) {
                }
        })
 }
+
+func TestRemoveReadOnlyFile(t *testing.T) {
+       testMaybeRooted(t, func(t *testing.T, r *Root) {
+               if err := WriteFile("file", []byte("1"), 0); err != nil {
+                       t.Fatal(err)
+               }
+               var err error
+               if r == nil {
+                       err = Remove("file")
+               } else {
+                       err = r.Remove("file")
+               }
+               if err != nil {
+                       t.Fatalf("Remove read-only file: %v", err)
+               }
+               if _, err := Stat("file"); !IsNotExist(err) {
+                       t.Fatalf("Stat read-only file after removal: %v (want IsNotExist)", err)
+               }
+       })
+}
index 1574817098f34c82aeedb80266a56673c3ebe155..55455d2c941eeeec16d9cf72b0d847beb26e63bd 100644 (file)
@@ -120,6 +120,12 @@ func (r *Root) Mkdir(name string, perm FileMode) error {
        return rootMkdir(r, name, perm)
 }
 
+// Remove removes the named file or (empty) directory in the root.
+// See [Remove] for more details.
+func (r *Root) Remove(name string) error {
+       return rootRemove(r, name)
+}
+
 func (r *Root) logOpen(name string) {
        if log := testlog.Logger(); log != nil {
                // This won't be right if r's name has changed since it was opened,
index 72138d1e8995c6f50350bd88dba30662c9630bd1..70aa5f9ccd0cd01479c20615866a52c26075ee3d 100644 (file)
@@ -12,7 +12,24 @@ import (
        "syscall"
 )
 
+// checkPathEscapes reports whether name escapes the root.
+//
+// Due to the lack of openat, checkPathEscapes is subject to TOCTOU races
+// when symlinks change during the resolution process.
 func checkPathEscapes(r *Root, name string) error {
+       return checkPathEscapesInternal(r, name, false)
+}
+
+// checkPathEscapesLstat reports whether name escapes the root.
+// It does not resolve symlinks in the final path component.
+//
+// Due to the lack of openat, checkPathEscapes is subject to TOCTOU races
+// when symlinks change during the resolution process.
+func checkPathEscapesLstat(r *Root, name string) error {
+       return checkPathEscapesInternal(r, name, true)
+}
+
+func checkPathEscapesInternal(r *Root, name string, lstat bool) error {
        if r.root.closed.Load() {
                return ErrClosed
        }
@@ -44,6 +61,10 @@ func checkPathEscapes(r *Root, name string) error {
                        continue
                }
 
+               if lstat && i == len(parts)-1 {
+                       break
+               }
+
                next := joinPath(base, parts[i])
                fi, err := Lstat(next)
                if err != nil {
index be7f5507eb8c0497aa8840095572ba09d6df23af..d59720a7b7537a9812376a23aafe57375fa814bc 100644 (file)
@@ -84,3 +84,13 @@ func rootMkdir(r *Root, name string, perm FileMode) error {
        }
        return nil
 }
+
+func rootRemove(r *Root, name string) error {
+       if err := checkPathEscapesLstat(r, name); err != nil {
+               return &PathError{Op: "removeat", Path: name, Err: err}
+       }
+       if err := Remove(joinPath(r.root.name, name)); err != nil {
+               return &PathError{Op: "removeat", Path: name, Err: underlyingError(err)}
+       }
+       return nil
+}
index 7f6619bab41efcdf4769e21c9aa83e8317c6025c..a03208b4c170e9b511e6b24f4593bfef0ccab6b2 100644 (file)
@@ -74,6 +74,16 @@ func rootMkdir(r *Root, name string, perm FileMode) error {
        return err
 }
 
+func rootRemove(r *Root, name string) error {
+       _, err := doInRoot(r, name, func(parent sysfdType, name string) (struct{}, error) {
+               return struct{}{}, removeat(parent, name)
+       })
+       if err != nil {
+               return &PathError{Op: "removeat", Path: name, Err: err}
+       }
+       return err
+}
+
 // doInRoot performs an operation on a path in a Root.
 //
 // It opens the directory containing the final element of the path,
index 0a26e7352af5a88126e417743a0ebe41eedf4ff6..08005accb54be123001f6373a653b38affe71abb 100644 (file)
@@ -19,3 +19,7 @@ func checkPathEscapes(r *Root, name string) error {
        }
        return nil
 }
+
+func checkPathEscapesLstat(r *Root, name string) error {
+       return checkPathEscapes(r, name)
+}
index 1edccf362c7bc9204000c14d45c363cdfa392fa9..70c378cef8d43d3e59de5bf0a1e5bca22ed278e8 100644 (file)
@@ -105,6 +105,13 @@ type rootTest struct {
        // the target is the filename that should not have been opened.
        target string
 
+       // ltarget is the filename that we expect to accessed, after resolving all symlinks
+       // except the last one. This is the file we expect to be removed by Remove or statted
+       // by Lstat.
+       //
+       // If the last path component in open is not a symlink, ltarget should be "".
+       ltarget string
+
        // wantError is true if accessing the file should fail.
        wantError bool
 
@@ -176,8 +183,9 @@ var rootTestCases = []rootTest{{
        fs: []string{
                "link => target",
        },
-       open:   "link",
-       target: "target",
+       open:    "link",
+       target:  "target",
+       ltarget: "link",
 }, {
        name: "symlink chain",
        fs: []string{
@@ -188,8 +196,9 @@ var rootTestCases = []rootTest{{
                "g/h/i => ..",
                "g/c/",
        },
-       open:   "link",
-       target: "g/c/target",
+       open:    "link",
+       target:  "g/c/target",
+       ltarget: "link",
 }, {
        name: "path with dot",
        fs: []string{
@@ -251,6 +260,7 @@ var rootTestCases = []rootTest{{
                "a => a",
        },
        open:        "a",
+       ltarget:     "a",
        wantError:   true,
        alwaysFails: true,
 }, {
@@ -273,6 +283,7 @@ var rootTestCases = []rootTest{{
                "link => $ABS/target",
        },
        open:      "link",
+       ltarget:   "link",
        target:    "target",
        wantError: true,
 }, {
@@ -282,6 +293,7 @@ var rootTestCases = []rootTest{{
        },
        open:      "link",
        target:    "target",
+       ltarget:   "link",
        wantError: true,
 }, {
        name: "symlink chain escapes",
@@ -293,6 +305,7 @@ var rootTestCases = []rootTest{{
        },
        open:      "link",
        target:    "c/target",
+       ltarget:   "link",
        wantError: true,
 }}
 
@@ -421,6 +434,60 @@ func TestRootOpenRoot(t *testing.T) {
        }
 }
 
+func TestRootRemoveFile(t *testing.T) {
+       for _, test := range rootTestCases {
+               test.run(t, func(t *testing.T, target string, root *os.Root) {
+                       wantError := test.wantError
+                       if test.ltarget != "" {
+                               // Remove doesn't follow symlinks in the final path component,
+                               // so it will successfully remove ltarget.
+                               wantError = false
+                               target = filepath.Join(root.Name(), test.ltarget)
+                       } else if target != "" {
+                               if err := os.WriteFile(target, nil, 0o666); err != nil {
+                                       t.Fatal(err)
+                               }
+                       }
+
+                       err := root.Remove(test.open)
+                       if errEndsTest(t, err, wantError, "root.Remove(%q)", test.open) {
+                               return
+                       }
+                       _, err = os.Lstat(target)
+                       if !errors.Is(err, os.ErrNotExist) {
+                               t.Fatalf(`stat file removed with Root.Remove(%q): %v, want ErrNotExist`, test.open, err)
+                       }
+               })
+       }
+}
+
+func TestRootRemoveDirectory(t *testing.T) {
+       for _, test := range rootTestCases {
+               test.run(t, func(t *testing.T, target string, root *os.Root) {
+                       wantError := test.wantError
+                       if test.ltarget != "" {
+                               // Remove doesn't follow symlinks in the final path component,
+                               // so it will successfully remove ltarget.
+                               wantError = false
+                               target = filepath.Join(root.Name(), test.ltarget)
+                       } else if target != "" {
+                               if err := os.Mkdir(target, 0o777); err != nil {
+                                       t.Fatal(err)
+                               }
+                       }
+
+                       err := root.Remove(test.open)
+                       if errEndsTest(t, err, wantError, "root.Remove(%q)", test.open) {
+                               return
+                       }
+                       _, err = os.Lstat(target)
+                       if !errors.Is(err, os.ErrNotExist) {
+                               t.Fatalf(`stat file removed with Root.Remove(%q): %v, want ErrNotExist`, test.open, err)
+                       }
+               })
+       }
+}
+
 func TestRootOpenFileAsRoot(t *testing.T) {
        dir := t.TempDir()
        target := filepath.Join(dir, "target")
@@ -733,6 +800,23 @@ func TestRootConsistencyMkdir(t *testing.T) {
        }
 }
 
+func TestRootConsistencyRemove(t *testing.T) {
+       for _, test := range rootConsistencyTestCases {
+               if test.open == "." || test.open == "./" {
+                       continue // can't remove the root itself
+               }
+               test.run(t, func(t *testing.T, path string, r *os.Root) (string, error) {
+                       var err error
+                       if r == nil {
+                               err = os.Remove(path)
+                       } else {
+                               err = r.Remove(path)
+                       }
+                       return "", err
+               })
+       }
+}
+
 func TestRootRenameAfterOpen(t *testing.T) {
        switch runtime.GOOS {
        case "windows":
index 496a11903b8a758d8dc550c682f8c09992b5c62a..6f8f9c8e3ea1dc99eedcbb0206bf9d9201728195 100644 (file)
@@ -119,6 +119,28 @@ func mkdirat(fd int, name string, perm FileMode) error {
        })
 }
 
+func removeat(fd int, name string) error {
+       // The system call interface forces us to know whether
+       // we are removing a file or directory. Try both.
+       e := ignoringEINTR(func() error {
+               return unix.Unlinkat(fd, name, 0)
+       })
+       if e == nil {
+               return nil
+       }
+       e1 := ignoringEINTR(func() error {
+               return unix.Unlinkat(fd, name, unix.AT_REMOVEDIR)
+       })
+       if e1 == nil {
+               return nil
+       }
+       // Both failed. See comment in Remove for how we decide which error to return.
+       if e1 != syscall.ENOTDIR {
+               return e1
+       }
+       return e
+}
+
 // checkSymlink resolves the symlink name in parent,
 // and returns errSymlink with the link contents.
 //
index 685737ea44ca100ef80af0ea02dcb75227d90f11..68f938de939891fe3514e34ec5e938d50f67b311 100644 (file)
@@ -201,3 +201,7 @@ func rootOpenDir(parent syscall.Handle, name string) (syscall.Handle, error) {
 func mkdirat(dirfd syscall.Handle, name string, perm FileMode) error {
        return windows.Mkdirat(dirfd, name, syscallMode(perm))
 }
+
+func removeat(dirfd syscall.Handle, name string) error {
+       return windows.Deleteat(dirfd, name)
+}
index f9bddc0d672fb4d4b16717568220a42b4d46b60b..62e20971231f501341aec9ac5d7b4d727c1aa8ec 100644 (file)
@@ -7,6 +7,7 @@
 package os_test
 
 import (
+       "errors"
        "os"
        "path/filepath"
        "testing"
@@ -43,4 +44,10 @@ func TestRootWindowsCaseInsensitivity(t *testing.T) {
                t.Fatal(err)
        }
        f.Close()
+       if err := r.Remove("FILE"); err != nil {
+               t.Fatal(err)
+       }
+       if _, err := os.Stat(filepath.Join(dir, "file")); !errors.Is(err, os.ErrNotExist) {
+               t.Fatalf("os.Stat(file) after deletion: %v, want ErrNotFound", err)
+       }
 }