fusefrontend: -allow_other: Use MkdiratUser in Mkdir FUSE call.

Revert commit fcaca5fc94d981aa637beb752edc8cb3c2265e96.

Instead of manually adjusting the user and mode after creating the
directory, adjust effective permissions and let the kernel deal with it.

Related to https://github.com/rfjakob/gocryptfs/issues/338.

internal/fusefrontend/fs_dir.go

@@ -24,13 +24,13 @@ const dsStoreName = ".DS_Store"
 // mkdirWithIv - create a new directory and corresponding diriv file. dirfd
 // should be a handle to the parent directory, cName is the name of the new
 // directory and mode specifies the access permissions to use.
-func (fs *FS) mkdirWithIv(dirfd int, cName string, mode uint32) error {
+func (fs *FS) mkdirWithIv(dirfd int, cName string, mode uint32, context *fuse.Context) error {
 	// Between the creation of the directory and the creation of gocryptfs.diriv
 	// the directory is inconsistent. Take the lock to prevent other readers
 	// from seeing it.
 	fs.dirIVLock.Lock()
 	defer fs.dirIVLock.Unlock()
-	err := syscallcompat.Mkdirat(dirfd, cName, mode)
+	err := syscallcompat.MkdiratUser(dirfd, cName, mode, context)
 	if err != nil {
 		return err
 	}
@@ -63,40 +63,20 @@ func (fs *FS) Mkdir(newPath string, mode uint32, context *fuse.Context) (code fu
 		return fuse.ToStatus(err)
 	}
 	defer syscall.Close(dirfd)
-	// Don't set the full mode before we have set the correct owner.
+	// Make sure context is nil if we don't want to preserve the owner
-	// See Create FUSE call for more details.
+	if !fs.args.PreserveOwner {
-	origMode := mode
+		context = nil
-	if fs.args.PreserveOwner {
-		mode = 0000
 	}
 	if fs.args.PlaintextNames {
-		err = syscallcompat.Mkdirat(dirfd, cName, mode)
+		err = syscallcompat.MkdiratUser(dirfd, cName, mode, context)
-		if err != nil {
 		return fuse.ToStatus(err)
 	}
-		// Set owner
-		if fs.args.PreserveOwner {
-			err = syscallcompat.Fchownat(dirfd, cName, int(context.Owner.Uid),
-				int(context.Owner.Gid), unix.AT_SYMLINK_NOFOLLOW)
-			if err != nil {
-				tlog.Warn.Printf("Mkdir %q: Fchownat %d:%d failed: %v", cName, context.Owner.Uid, context.Owner.Gid, err)
-				// In case of a failure, we don't want to proceed setting more
-				// permissive modes.
-				return fuse.ToStatus(err)
-			}
-		}
-		// Set mode
-		if origMode != mode {
-			err = syscallcompat.Fchmodat(dirfd, cName, origMode, unix.AT_SYMLINK_NOFOLLOW)
-			if err != nil {
-				tlog.Warn.Printf("Mkdir %q: Fchmodat %#o -> %#o failed: %v", cName, mode, origMode, err)
-			}
-		}
-		return fuse.OK
-	}
 
-	// We need write and execute permissions to create gocryptfs.diriv
+	// We need write and execute permissions to create gocryptfs.diriv.
-	mode = mode | 0300
+	// Also, we need read permissions to open the directory (to avoid
+	// race-conditions between getting and setting the mode).
+	origMode := mode
+	mode = mode | 0700
 
 	// Handle long file name
 	if nametransform.IsLongContent(cName) {
@@ -107,33 +87,40 @@ func (fs *FS) Mkdir(newPath string, mode uint32, context *fuse.Context) (code fu
 		}
 
 		// Create directory
-		err = fs.mkdirWithIv(dirfd, cName, mode)
+		err = fs.mkdirWithIv(dirfd, cName, mode, context)
 		if err != nil {
 			nametransform.DeleteLongNameAt(dirfd, cName)
 			return fuse.ToStatus(err)
 		}
 	} else {
-		err = fs.mkdirWithIv(dirfd, cName, mode)
+		err = fs.mkdirWithIv(dirfd, cName, mode, context)
 		if err != nil {
 			return fuse.ToStatus(err)
 		}
 	}
-	// Set owner
-	if fs.args.PreserveOwner {
-		err = syscallcompat.Fchownat(dirfd, cName, int(context.Owner.Uid),
-			int(context.Owner.Gid), unix.AT_SYMLINK_NOFOLLOW)
-		if err != nil {
-			tlog.Warn.Printf("Mkdir %q: Fchownat %d:%d failed: %v", cName, context.Owner.Uid, context.Owner.Gid, err)
-			// In case of a failure, we don't want to proceed setting more
-			// permissive modes.
-			return fuse.ToStatus(err)
-		}
-	}
 	// Set mode
 	if origMode != mode {
-		err = syscallcompat.Fchmodat(dirfd, cName, origMode, unix.AT_SYMLINK_NOFOLLOW)
+		dirfd2, err := syscallcompat.Openat(dirfd, cName,
+			syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW, 0)
 		if err != nil {
-			tlog.Warn.Printf("Mkdir %q: Fchmodat %#o -> %#o failed: %v", cName, mode, origMode, err)
+			tlog.Warn.Printf("Mkdir %q: Openat failed: %v", cName, err)
+			return fuse.ToStatus(err)
+		}
+		defer syscall.Close(dirfd2)
+
+		var st syscall.Stat_t
+		err = syscall.Fstat(dirfd2, &st)
+		if err != nil {
+			tlog.Warn.Printf("Mkdir %q: Fstat failed: %v", cName, err)
+			return fuse.ToStatus(err)
+		}
+
+		// Preserve SGID bit if it was set due to inheritance.
+		origMode = uint32(st.Mode&^0777) | origMode
+		err = syscall.Fchmod(dirfd2, origMode)
+		if err != nil {
+			tlog.Warn.Printf("Mkdir %q: Fchmod %#o -> %#o failed: %v", cName, mode, origMode, err)
+			return fuse.ToStatus(err)
 		}
 	}
 	return fuse.OK

internal/syscallcompat/sys_darwin.go

@@ -79,6 +79,11 @@ func Mkdirat(dirfd int, path string, mode uint32) (err error) {
 	return emulateMkdirat(dirfd, path, mode)
 }
 
+func MkdiratUser(dirfd int, path string, mode uint32, context *fuse.Context) (err error) {
+	// FIXME: take into account context.Owner
+	return Mkdirat(dirfd, path, mode)
+}
+
 func Fstatat(dirfd int, path string, stat *unix.Stat_t, flags int) (err error) {
 	return emulateFstatat(dirfd, path, stat, flags)
 }

internal/syscallcompat/sys_linux.go

@@ -180,6 +180,28 @@ func Mkdirat(dirfd int, path string, mode uint32) (err error) {
 	return syscall.Mkdirat(dirfd, path, mode)
 }
 
+// MkdiratUser runs the Mkdirat syscall in the context of a different user.
+func MkdiratUser(dirfd int, path string, mode uint32, context *fuse.Context) (err error) {
+	if context != nil {
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+
+		err = syscall.Setregid(-1, int(context.Owner.Gid))
+		if err != nil {
+			return err
+		}
+		defer syscall.Setregid(-1, 0)
+
+		err = syscall.Setreuid(-1, int(context.Owner.Uid))
+		if err != nil {
+			return err
+		}
+		defer syscall.Setreuid(-1, 0)
+	}
+
+	return Mkdirat(dirfd, path, mode)
+}
+
 // Fstatat syscall.
 func Fstatat(dirfd int, path string, stat *unix.Stat_t, flags int) (err error) {
 	// Why would we ever want to call this without AT_SYMLINK_NOFOLLOW?