goferfs: Return EOPNOTSUPP when attempting to change mode for symlinks.

As of writing, gVisor does not support fchmodat2(2). Without it, there is no
way to change permissions for a symlink file. Note that fchmod(2) on an O_PATH
FD fails with EBADF. Also fchmodat(2) (which is supported) does not take any
flags and AT_SYMLINK_NOFOLLOW is needed to change permissions on a symlink.

But if in the future we were to support fchmodat2(2), goferfs would return
EBADF because it would try to change mode using fchmod(2) on a O_PATH FD.
Updated goferfs to explicitly return EOPNOTSUPP (consistent with Linux).

Some other minor fixes:
- The comment about why sockets needed special handling for changing mode was
  incorrect. Sockets too have a O_PATH control FD so fchmod(2) on it would fail
  with EBADF. This was the real reason.
- The comment about AT_SYMLINK_NOFOLLOW not being supported is stale. Support
  for fchmodat2(2) along with AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH flags was
  added in Linux 6.6. Removed the comment for now. We should probably use
  fchmodat2(2) when it is available because it is safer.
- When --directfs=true and the application tries to change mode of a mountpoint
  socket, gofer client makes an RPC to the fsgofer server to do that work.
  fsgofer attempts the mode change using fchmodat(2). However, the fsgofer's
  seccomp filters did not allow fchmodat(2) in --directfs=true mode. Added
  fchmodat(2) to fsgofer seccomp filters unconditionally.

Updates #10385

PiperOrigin-RevId: 687385378

pkg/sentry/fsimpl/gofer/directfs_dentry.go

@@ -266,24 +266,25 @@ func (d *directfsDentry) updateMetadataLocked(h handle) error {
 
 // Precondition: fs.renameMu is locked if d is a socket.
 func (d *directfsDentry) chmod(ctx context.Context, mode uint16) error {
+	if d.isSymlink() {
+		// Linux does not support changing the mode of symlinks. See
+		// fs/attr.c:notify_change().
+		return unix.EOPNOTSUPP
+	}
 	if !d.isSocket() {
 		return unix.Fchmod(d.controlFD, uint32(mode))
 	}
 
-	// fchmod(2) on socket files created via bind(2) fails. We need to
-	// fchmodat(2) it from its parent.
+	// Sockets use O_PATH control FDs. However, fchmod(2) fails with EBADF for
+	// O_PATH FDs. Try to fchmodat(2) it from its parent.
 	if parent := d.parent.Load(); parent != nil {
-		// We have parent FD, just use that. Note that AT_SYMLINK_NOFOLLOW flag is
-		// currently not supported. So we don't use it.
 		return unix.Fchmodat(parent.impl.(*directfsDentry).controlFD, d.name, uint32(mode), 0 /* flags */)
 	}
 
-	// This is a mount point socket. We don't have a parent FD. Fallback to using
-	// lisafs.
-	if !d.controlFDLisa.Ok() {
-		panic("directfsDentry.controlFDLisa is not set for mount point socket")
+	// This is a mount point socket (no parent). Fallback to using lisafs.
+	if err := d.ensureLisafsControlFD(ctx); err != nil {
+		return err
 	}
-
 	return chmod(ctx, d.controlFDLisa, mode)
 }
 

runsc/fsgofer/filter/config.go

@@ -43,6 +43,7 @@ var allowedSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{
 	unix.SYS_EXIT:       seccomp.MatchAll{},
 	unix.SYS_EXIT_GROUP: seccomp.MatchAll{},
 	unix.SYS_FCHMOD:     seccomp.MatchAll{},
+	unix.SYS_FCHMODAT:   seccomp.MatchAll{},
 	unix.SYS_FCHOWNAT:   seccomp.MatchAll{},
 	unix.SYS_FCNTL: seccomp.Or{
 		seccomp.PerArg{
@@ -221,7 +222,6 @@ var lisafsFilters = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{
 		seccomp.AnyValue{},
 		seccomp.EqualTo(0),
 	},
-	unix.SYS_FCHMODAT:   seccomp.MatchAll{},
 	unix.SYS_FGETXATTR:  seccomp.MatchAll{},
 	unix.SYS_FSTATFS:    seccomp.MatchAll{},
 	unix.SYS_GETDENTS64: seccomp.MatchAll{},

runsc/fsgofer/lisafs.go

@@ -288,12 +288,17 @@ func (fd *controlFDLisa) Stat() (linux.Statx, error) {
 // SetStat implements lisafs.ControlFDImpl.SetStat.
 func (fd *controlFDLisa) SetStat(stat lisafs.SetStatReq) (failureMask uint32, failureErr error) {
 	if stat.Mask&unix.STATX_MODE != 0 {
-		if fd.IsSocket() {
-			// fchmod(2) on socket files created via bind(2) fails. We need to
-			// fchmodat(2) it from its parent.
+		switch fd.FileType() {
+		case unix.S_IFLNK:
+			// Linux does not support changing the mode of symlinks. See
+			// fs/attr.c:notify_change().
+			failureMask |= unix.STATX_MODE
+			failureErr = unix.EOPNOTSUPP
+		case unix.S_IFSOCK:
+			// Sockets use O_PATH host FDs. However, fchmod(2) fails with EBADF for
+			// O_PATH FDs. Try to fchmodat(2) it from its parent.
 			parent, sockName, err := fd.getParentFD()
 			if err == nil {
-				// Note that AT_SYMLINK_NOFOLLOW flag is not currently supported.
 				err = unix.Fchmodat(parent, sockName, stat.Mode&^unix.S_IFMT, 0 /* flags */)
 				unix.Close(parent)
 			}
@@ -302,7 +307,7 @@ func (fd *controlFDLisa) SetStat(stat lisafs.SetStatReq) (failureMask uint32, fa
 				failureMask |= unix.STATX_MODE
 				failureErr = err
 			}
-		} else {
+		default:
 			if err := unix.Fchmod(fd.hostFD, stat.Mode&^unix.S_IFMT); err != nil {
 				log.Warningf("SetStat fchmod failed %q, err: %v", fd.Node().FilePath(), err)
 				failureMask |= unix.STATX_MODE