summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorDean Deng <deandeng@google.com>2020-09-15 14:50:45 -0700
committergVisor bot <gvisor-bot@google.com>2020-09-15 14:55:28 -0700
commita004f0d082458ce98114b59a3e5d41c2e74541d6 (patch)
tree22d790e0c00ef6968f6d5d6e946cb5d4db95ad6c
parent72a30b11486b48394fa0edca500b80e4ca83b10c (diff)
Support setting STATX_SIZE for kernfs.InodeAttrs.
Make setting STATX_SIZE a no-op, if it is valid for the given permissions and file type. Also update proc tests, which were overfitted before. Fixes #3842. Updates #1193. PiperOrigin-RevId: 331861087
-rw-r--r--pkg/sentry/fsimpl/kernfs/inode_impl_util.go19
-rw-r--r--test/syscalls/linux/proc.cc23
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());
}
}