Open directories using O_SEARCH

Right now we open directories using O_RDONLY. This is problematic, because it means we can't perform operations on directories that only have execute permissions. The solution for this is to use O_SEARCH, which was added in POSIX 2008. Linux doesn't offer O_SEARCH, but we can emulate it using O_PATH. macOS has supported this feature since Ventura (October 2022).
buildbarn · Dec 5, 2024 · b263fc6 · b263fc6
1 parent f5a181e
commit b263fc6
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 3 deletions.
diff --git a/MODULE.bazel b/MODULE.bazel
@@ -99,6 +99,10 @@ go_deps_dev.module_override(
     patches = ["//:patches/org_golang_x_oauth2/injectable-clock.diff"],
     path = "golang.org/x/oauth2",
 )
+go_deps_dev.module_override(
+    patches = ["//:patches/org_golang_x_sys/o-search.diff"],
+    path = "golang.org/x/sys",
+)
 go_deps_dev.module_override(
     patches = ["//:patches/org_uber_go_mock/mocks-for-funcs.diff"],
     path = "go.uber.org/mock",

diff --git a/patches/org_golang_x_sys/o-search.diff b/patches/org_golang_x_sys/o-search.diff
@@ -0,0 +1,40 @@
+diff --git unix/zerrors_darwin_amd64.go unix/zerrors_darwin_amd64.go
+index d73c465..d406964 100644
+--- unix/zerrors_darwin_amd64.go
++++ unix/zerrors_darwin_amd64.go
+@@ -1128,6 +1128,7 @@ const (
+ 	O_DSYNC                                 = 0x400000
+ 	O_EVTONLY                               = 0x8000
+ 	O_EXCL                                  = 0x800
++	O_EXEC                                  = 0x40000000
+ 	O_EXLOCK                                = 0x20
+ 	O_FSYNC                                 = 0x80
+ 	O_NDELAY                                = 0x4
+@@ -1138,6 +1139,7 @@ const (
+ 	O_POPUP                                 = 0x80000000
+ 	O_RDONLY                                = 0x0
+ 	O_RDWR                                  = 0x2
++	O_SEARCH                                = 0x40100000
+ 	O_SHLOCK                                = 0x10
+ 	O_SYMLINK                               = 0x200000
+ 	O_SYNC                                  = 0x80
+diff --git unix/zerrors_darwin_arm64.go unix/zerrors_darwin_arm64.go
+index 4a55a40..c47c6e9 100644
+--- unix/zerrors_darwin_arm64.go
++++ unix/zerrors_darwin_arm64.go
+@@ -1128,6 +1128,7 @@ const (
+ 	O_DSYNC                                 = 0x400000
+ 	O_EVTONLY                               = 0x8000
+ 	O_EXCL                                  = 0x800
++	O_EXEC                                  = 0x40000000
+ 	O_EXLOCK                                = 0x20
+ 	O_FSYNC                                 = 0x80
+ 	O_NDELAY                                = 0x4
+@@ -1138,6 +1139,7 @@ const (
+ 	O_POPUP                                 = 0x80000000
+ 	O_RDONLY                                = 0x0
+ 	O_RDWR                                  = 0x2
++	O_SEARCH                                = 0x40100000
+ 	O_SHLOCK                                = 0x10
+ 	O_SYMLINK                               = 0x200000
+ 	O_SYNC                                  = 0x80
diff --git a/pkg/filesystem/local_directory_darwin.go b/pkg/filesystem/local_directory_darwin.go
@@ -16,6 +16,8 @@ import (
 // rawDeviceNumber is the equivalent of POSIX dev_t.
 type rawDeviceNumber = int32
 
+const oflagSearch = unix.O_SEARCH
+
 func (d *localDirectory) Mknod(name path.Component, perm os.FileMode, deviceNumber DeviceNumber) error {
 	return status.Error(codes.Unimplemented, "Creation of device nodes is not supported on Darwin")
 }

diff --git a/pkg/filesystem/local_directory_freebsd.go b/pkg/filesystem/local_directory_freebsd.go
@@ -15,6 +15,8 @@ import (
 // rawDeviceNumber is the equivalent of POSIX dev_t.
 type rawDeviceNumber = uint64
 
+const oflagSearch = unix.O_SEARCH
+
 func (d *localDirectory) Mknod(name path.Component, perm os.FileMode, deviceNumber DeviceNumber) error {
 	// Though mknodat() exists on FreeBSD, device nodes created
 	// outside of devfs are non-functional.

diff --git a/pkg/filesystem/local_directory_linux.go b/pkg/filesystem/local_directory_linux.go
@@ -18,6 +18,8 @@ import (
 // rawDeviceNumber is the equivalent of POSIX dev_t.
 type rawDeviceNumber = uint64
 
+const oflagSearch = unix.O_PATH
+
 func (d *localDirectory) Mknod(name path.Component, perm os.FileMode, deviceNumber DeviceNumber) error {
 	defer runtime.KeepAlive(d)
 

diff --git a/pkg/filesystem/local_directory_unix.go b/pkg/filesystem/local_directory_unix.go
@@ -43,7 +43,7 @@ func NewLocalDirectory(directoryParser path.Parser) (DirectoryCloser, error) {
 		return nil, util.StatusWrap(err, "Failed to create local representation of directory")
 	}
 
-	fd, err := unix.Openat(unix.AT_FDCWD, pathString, unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_RDONLY, 0)
+	fd, err := unix.Openat(unix.AT_FDCWD, pathString, unix.O_DIRECTORY|unix.O_NOFOLLOW|oflagSearch, 0)
 	if err != nil {
 		return nil, err
 	}
@@ -53,7 +53,7 @@ func NewLocalDirectory(directoryParser path.Parser) (DirectoryCloser, error) {
 func (d *localDirectory) enter(name path.Component) (*localDirectory, error) {
 	defer runtime.KeepAlive(d)
 
-	fd, err := unix.Openat(d.fd, name.String(), unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_RDONLY, 0)
+	fd, err := unix.Openat(d.fd, name.String(), unix.O_DIRECTORY|unix.O_NOFOLLOW|oflagSearch, 0)
 	if err != nil {
 		if runtime.GOOS == "freebsd" && err == syscall.EMLINK {
 			// FreeBSD erroneously returns EMLINK.
@@ -406,7 +406,19 @@ func (d *localDirectory) Symlink(oldName path.Parser, newName path.Component) er
 func (d *localDirectory) Sync() error {
 	defer runtime.KeepAlive(d)
 
-	return unix.Fsync(d.fd)
+	// Linux doesn't permit calling fsync() on directories opened
+	// with O_PATH. Reopen the directory with O_RDONLY.
+	dfd := d.fd
+	if runtime.GOOS == "linux" {
+		var err error
+		dfd, err = unix.Openat(d.fd, ".", unix.O_DIRECTORY|unix.O_RDONLY, 0)
+		if err != nil {
+			return err
+		}
+		defer unix.Close(dfd)
+	}
+
+	return unix.Fsync(dfd)
 }
 
 func (d *localDirectory) Chtimes(name path.Component, atime, mtime time.Time) error {