diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go index e8f3033..7492778 100644 --- a/internal/fusefrontend/fs.go +++ b/internal/fusefrontend/fs.go @@ -290,7 +290,7 @@ func (fs *FS) Chmod(path string, mode uint32, context *fuse.Context) (code fuse. defer syscall.Close(dirfd) // os.Chmod goes through the "syscallMode" translation function that messes // up the suid and sgid bits. So use a syscall directly. - err = syscallcompat.Fchmodat(dirfd, cName, mode, unix.AT_SYMLINK_NOFOLLOW) + err = syscallcompat.FchmodatNofollow(dirfd, cName, mode) return fuse.ToStatus(err) } diff --git a/internal/fusefrontend/fs_dir.go b/internal/fusefrontend/fs_dir.go index a017050..d26fd79 100644 --- a/internal/fusefrontend/fs_dir.go +++ b/internal/fusefrontend/fs_dir.go @@ -164,7 +164,7 @@ func (fs *FS) Rmdir(relPath string, context *fuse.Context) (code fuse.Status) { } // This cast is needed on Darwin, where st.Mode is uint16. origMode := uint32(st.Mode) - err = syscallcompat.Fchmodat(parentDirFd, cName, origMode|0700, unix.AT_SYMLINK_NOFOLLOW) + err = syscallcompat.FchmodatNofollow(parentDirFd, cName, origMode|0700) if err != nil { tlog.Debug.Printf("Rmdir: Fchmodat failed: %v", err) return fuse.ToStatus(err) @@ -175,7 +175,7 @@ func (fs *FS) Rmdir(relPath string, context *fuse.Context) (code fuse.Status) { // Undo the chmod if removing the directory failed defer func() { if code != fuse.OK { - err = syscallcompat.Fchmodat(parentDirFd, cName, origMode, unix.AT_SYMLINK_NOFOLLOW) + err = syscallcompat.FchmodatNofollow(parentDirFd, cName, origMode) if err != nil { tlog.Warn.Printf("Rmdir: Chmod rollback failed: %v", err) } diff --git a/internal/syscallcompat/sys_common_test.go b/internal/syscallcompat/sys_common_test.go index 14e1b3f..3729111 100644 --- a/internal/syscallcompat/sys_common_test.go +++ b/internal/syscallcompat/sys_common_test.go @@ -175,7 +175,7 @@ func TestFchmodat(t *testing.T) { defer syscall.Close(dirfd) // Check that chmod on a regular file works ok - err = Fchmodat(dirfd, regular, 0111, 0) + err = FchmodatNofollow(dirfd, regular, 0111, 0) if err != nil { t.Fatal(err) } @@ -185,7 +185,7 @@ func TestFchmodat(t *testing.T) { if st.Mode != 0111 { t.Errorf("wrong mode: %#0o", st.Mode) } - err = Fchmodat(dirfd, regular, 0000, 0) + err = FchmodatNofollow(dirfd, regular, 0000, 0) if err != nil { t.Error(err) } @@ -196,7 +196,7 @@ func TestFchmodat(t *testing.T) { } // Check what happens on a symlink - err = Fchmodat(dirfd, symlink, 0333, 0) + err = FchmodatNofollow(dirfd, symlink, 0333, 0) if err == nil { syscall.Lstat(tmpDir+"/"+symlink, &st) st.Mode &= 0777 diff --git a/internal/syscallcompat/sys_darwin.go b/internal/syscallcompat/sys_darwin.go index b8b82c8..ebda80f 100644 --- a/internal/syscallcompat/sys_darwin.go +++ b/internal/syscallcompat/sys_darwin.go @@ -8,8 +8,6 @@ import ( "golang.org/x/sys/unix" "github.com/hanwen/go-fuse/fuse" - - "github.com/rfjakob/gocryptfs/internal/tlog" ) const ( @@ -93,13 +91,8 @@ func MknodatUser(dirfd int, path string, mode uint32, dev int, context *fuse.Con return Mknodat(dirfd, path, mode, dev) } -func Fchmodat(dirfd int, path string, mode uint32, flags int) (err error) { - // Why would we ever want to call this without AT_SYMLINK_NOFOLLOW? - if flags&unix.AT_SYMLINK_NOFOLLOW == 0 { - tlog.Warn.Printf("Fchmodat: adding missing AT_SYMLINK_NOFOLLOW flag") - flags |= unix.AT_SYMLINK_NOFOLLOW - } - return unix.Fchmodat(dirfd, path, mode, flags) +func FchmodatNofollow(dirfd int, path string, mode uint32) (err error) { + return unix.Fchmodat(dirfd, path, mode, unix.AT_SYMLINK_NOFOLLOW) } func SymlinkatUser(oldpath string, newdirfd int, newpath string, context *fuse.Context) (err error) { diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go index e662001..625e136 100644 --- a/internal/syscallcompat/sys_linux.go +++ b/internal/syscallcompat/sys_linux.go @@ -113,20 +113,17 @@ func Dup3(oldfd int, newfd int, flags int) (err error) { return syscall.Dup3(oldfd, newfd, flags) } -// Fchmodat syscall. -func Fchmodat(dirfd int, path string, mode uint32, flags int) (err error) { - // Linux does not support passing flags to Fchmodat! From the man page: - // AT_SYMLINK_NOFOLLOW ... This flag is not currently implemented. - // Linux ignores any flags, but Go stdlib rejects them with EOPNOTSUPP starting - // with Go 1.11. See https://github.com/golang/go/issues/20130 for more info. - // TODO: Use fchmodat2 once available on Linux. - - // Why would we ever want to call this without AT_SYMLINK_NOFOLLOW? - if flags&unix.AT_SYMLINK_NOFOLLOW == 0 { - tlog.Warn.Printf("Fchmodat: adding missing AT_SYMLINK_NOFOLLOW flag") - } - +// FchmodatNofollow is like Fchmodat but never follows symlinks. +// +// This should be handled by the AT_SYMLINK_NOFOLLOW flag, but Linux +// does not implement it, so we have to perform an elaborate dance +// with O_PATH and /proc/self/fd. +// +// See also: Qemu implemented the same logic as fchmodat_nofollow(): +// https://git.qemu.org/?p=qemu.git;a=blob;f=hw/9pfs/9p-local.c#l335 +func FchmodatNofollow(dirfd int, path string, mode uint32) (err error) { // Open handle to the filename (but without opening the actual file). + // This succeeds even when we don't have read permissions to the file. fd, err := syscall.Openat(dirfd, path, syscall.O_NOFOLLOW|O_PATH, 0) if err != nil { return err @@ -134,22 +131,18 @@ func Fchmodat(dirfd int, path string, mode uint32, flags int) (err error) { defer syscall.Close(fd) // Now we can check the type without the risk of race-conditions. + // Return syscall.ELOOP if it is a symlink. var st syscall.Stat_t err = syscall.Fstat(fd, &st) if err != nil { return err } - - // Return syscall.ELOOP if path refers to a symlink. - var a fuse.Attr - a.FromStat(&st) - if a.IsSymlink() { + if st.Mode&syscall.S_IFMT == syscall.S_IFLNK { return syscall.ELOOP } - // Change mode of the actual file. Note that we can neither use - // Fchmodat (since fd is not necessarily a directory) nor Fchmod - // (since we are using O_PATH). + // Change mode of the actual file. Fchmod does not work with O_PATH, + // but Chmod via /proc/self/fd works. procPath := fmt.Sprintf("/proc/self/fd/%d", fd) return syscall.Chmod(procPath, mode) }