syscallcompat: rework Fchmodat to FchmodatNofollow

We never want Fchmodat to follow symlinks, so follow what
Qemu does, and call our function FchmodatNofollow.

5 files changed, 22 insertions, 36 deletions

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)
 }