diff options
| -rw-r--r-- | pkg/sentry/fsimpl/kernfs/inode_impl_util.go | 19 | ||||
| -rw-r--r-- | test/syscalls/linux/proc.cc | 23 |
2 files changed, 17 insertions, 25 deletions
diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index c0b863ba4..74408e322 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -259,9 +259,19 @@ func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *aut if opts.Stat.Mask == 0 { return nil } - if opts.Stat.Mask&^(linux.STATX_MODE|linux.STATX_UID|linux.STATX_GID) != 0 { + + // Note that not all fields are modifiable. For example, the file type and + // inode numbers are immutable after node creation. Setting the size is often + // allowed by kernfs files but does not do anything. If some other behavior is + // needed, the embedder should consider extending SetStat. + // + // TODO(gvisor.dev/issue/1193): Implement other stat fields like timestamps. + if opts.Stat.Mask&^(linux.STATX_MODE|linux.STATX_UID|linux.STATX_GID|linux.STATX_SIZE) != 0 { return syserror.EPERM } + if opts.Stat.Mask&linux.STATX_SIZE != 0 && a.Mode().IsDir() { + return syserror.EISDIR + } if err := vfs.CheckSetStat(ctx, creds, &opts, a.Mode(), auth.KUID(atomic.LoadUint32(&a.uid)), auth.KGID(atomic.LoadUint32(&a.gid))); err != nil { return err } @@ -284,13 +294,6 @@ func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *aut atomic.StoreUint32(&a.gid, stat.GID) } - // Note that not all fields are modifiable. For example, the file type and - // inode numbers are immutable after node creation. - - // TODO(gvisor.dev/issue/1193): Implement other stat fields like timestamps. - // Also, STATX_SIZE will need some special handling, because read-only static - // files should return EIO for truncate operations. - return nil } diff --git a/test/syscalls/linux/proc.cc b/test/syscalls/linux/proc.cc index f3f80a12a..1ba7bf4f5 100644 --- a/test/syscalls/linux/proc.cc +++ b/test/syscalls/linux/proc.cc @@ -797,17 +797,12 @@ TEST(ProcCpuinfo, DeniesWriteNonRoot) { constexpr int kNobody = 65534; EXPECT_THAT(syscall(SYS_setuid, kNobody), SyscallSucceeds()); EXPECT_THAT(open("/proc/cpuinfo", O_WRONLY), SyscallFailsWithErrno(EACCES)); - // TODO(gvisor.dev/issue/1193): Properly support setting size attributes in - // kernfs. - if (!IsRunningOnGvisor() || IsRunningWithVFS1()) { - EXPECT_THAT(truncate("/proc/cpuinfo", 123), - SyscallFailsWithErrno(EACCES)); - } + EXPECT_THAT(truncate("/proc/cpuinfo", 123), SyscallFailsWithErrno(EACCES)); }); } // With root privileges, it is possible to open /proc/cpuinfo with write mode, -// but all write operations will return EIO. +// but all write operations should fail. TEST(ProcCpuinfo, DeniesWriteRoot) { // VFS1 does not behave differently for root/non-root. SKIP_IF(IsRunningWithVFS1()); @@ -816,16 +811,10 @@ TEST(ProcCpuinfo, DeniesWriteRoot) { int fd; EXPECT_THAT(fd = open("/proc/cpuinfo", O_WRONLY), SyscallSucceeds()); if (fd > 0) { - EXPECT_THAT(write(fd, "x", 1), SyscallFailsWithErrno(EIO)); - EXPECT_THAT(pwrite(fd, "x", 1, 123), SyscallFailsWithErrno(EIO)); - } - // TODO(gvisor.dev/issue/1193): Properly support setting size attributes in - // kernfs. - if (!IsRunningOnGvisor() || IsRunningWithVFS1()) { - if (fd > 0) { - EXPECT_THAT(ftruncate(fd, 123), SyscallFailsWithErrno(EIO)); - } - EXPECT_THAT(truncate("/proc/cpuinfo", 123), SyscallFailsWithErrno(EIO)); + // Truncate is not tested--it may succeed on some kernels without doing + // anything. + EXPECT_THAT(write(fd, "x", 1), SyscallFails()); + EXPECT_THAT(pwrite(fd, "x", 1, 123), SyscallFails()); } } |