diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 5c54d0d56..4691f077b 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/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) } diff --git a/runsc/fsgofer/filter/config.go b/runsc/fsgofer/filter/config.go index 8f96bf584..3f6c150fd 100644 --- a/runsc/fsgofer/filter/config.go +++ b/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{}, diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index d28dfc047..3ee66deee 100644 --- a/runsc/fsgofer/lisafs.go +++ b/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