diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index 8dd2179796b..6c644933047 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -40,6 +40,24 @@ const ( cgroup2FsName = "cgroup2" ) +// mountLockFlags records which of a Mount's flags a remount may not clear. It +// is analogous to the MNT_LOCK_* flags in Linux. +// +// +stateify savable +type mountLockFlags struct { + // readOnly is analogous to MNT_LOCK_READONLY. + readOnly bool + + // noExec is analogous to MNT_LOCK_NOEXEC. + noExec bool + + // noDev is analogous to MNT_LOCK_NODEV. + noDev bool + + // noSUID is analogous to MNT_LOCK_NOSUID. + noSUID bool +} + // A Mount is a replacement of a Dentry (Mount.key.point) from one Filesystem // (Mount.key.parent.fs) with a Dentry (Mount.root) from another Filesystem // (Mount.fs), which applies to path resolution in the context of a particular @@ -123,6 +141,10 @@ type Mount struct { // namespace. It is analogous to MNT_LOCKED in Linux. locked bool + // lockedFlags contains the flags that RemountAt may not clear. lockedFlags + // is protected by VirtualFilesystem.mountMu. + lockedFlags mountLockFlags + // The lower 63 bits of writers is the number of calls to // Mount.CheckBeginWrite() that have not yet been paired with a call to // Mount.EndWrite(). The MSB of writers is set if MS_RDONLY is in effect. @@ -160,14 +182,31 @@ func (mnt *Mount) Options() MountOptions { } } +// canChangeLockedFlags returns false if applying opts to mnt would clear any +// of mnt's locked flags. +// +// Preconditions: +// - vfs.mountMu must be locked. +func (mnt *Mount) canChangeLockedFlags(opts *MountOptions) bool { + locked := mnt.lockedFlags + switch { + case locked.readOnly && !opts.ReadOnly: + return false + case locked.noExec && !opts.Flags.NoExec: + return false + case locked.noDev && !opts.Flags.NoDev: + return false + case locked.noSUID && !opts.Flags.NoSUID: + return false + } + return true +} + // setMountOptions sets mnt's options to the given opts. // // Preconditions: // - vfs.mountMu must be locked. func (mnt *Mount) setMountOptions(opts *MountOptions) error { - if opts == nil { - return linuxerr.EINVAL - } if err := mnt.setReadOnlyLocked(opts.ReadOnly); err != nil { return err } @@ -370,8 +409,11 @@ func (vfs *VirtualFilesystem) attachTreeLocked(ctx context.Context, mnt *Mount, // +checklocks:vfs.mountMu func (vfs *VirtualFilesystem) lockMountTree(mnt *Mount) { for _, m := range mnt.submountsLocked() { - // TODO(b/315839347): Add equivalents for MNT_LOCK_ATIME, - // MNT_LOCK_READONLY, etc. + // TODO(b/315839347): Add equivalents for MNT_LOCK_ATIME. + m.lockedFlags.readOnly = m.lockedFlags.readOnly || m.ReadOnlyLocked() + m.lockedFlags.noExec = m.lockedFlags.noExec || m.flags.NoExec + m.lockedFlags.noDev = m.lockedFlags.noDev || m.flags.NoDev + m.lockedFlags.noSUID = m.lockedFlags.noSUID || m.flags.NoSUID m.locked = true } } @@ -617,6 +659,7 @@ func (vfs *VirtualFilesystem) cloneMount(mnt *Mount, root *Dentry, mopts *MountO } clone.isShared = mnt.isShared clone.locked = mnt.locked + clone.lockedFlags = mnt.lockedFlags if cloneType&makeFollowerClone != 0 || (cloneType&sharedToFollowerClone != 0 && mnt.isShared) { mnt.followerList.PushFront(clone) clone.leader = mnt @@ -788,6 +831,9 @@ func (vfs *VirtualFilesystem) RemountAt(ctx context.Context, creds *auth.Credent if !vfs.validInMountNS(ctx, mnt) { return linuxerr.EINVAL } + if !mnt.canChangeLockedFlags(opts) { + return linuxerr.EPERM + } if err := mnt.setMountOptions(opts); err != nil { return err } diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 213936c4c5c..6e0ad48ec0b 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -2608,6 +2608,85 @@ TEST(MountTest, ChangeMountFlags) { ASSERT_EQ(st.f_flags & flag, 0); } +constexpr int kLockableFlags = MS_RDONLY | MS_NOEXEC | MS_NODEV | MS_NOSUID; + +// Sets kLockableFlags on the mount at `path`. +void SetLockableFlags(const std::string& path) { + ASSERT_THAT(mount("", path.c_str(), nullptr, + MS_REMOUNT | MS_BIND | kLockableFlags, ""), + SyscallSucceeds()); +} + +TEST(MountTest, ChangeMountFlagsOfLockedMount) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + const Cleanup mount_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, 0, "", 0)); + ASSERT_NO_FATAL_FAILURE(SetLockableFlags(dir.path())); + const std::string path = dir.path(); + const std::string file_path = JoinPath(path, "foo"); + + const std::function child = [&] { + // The mount was copied into a new user namespace, so none of its flags may + // be cleared. + for (const int flag : {MS_RDONLY, MS_NOEXEC, MS_NODEV, MS_NOSUID}) { + TEST_CHECK_ERRNO(mount("", path.c_str(), nullptr, + MS_REMOUNT | MS_BIND | (kLockableFlags & ~flag), + ""), + EPERM); + } + TEST_CHECK_ERRNO(mount("", path.c_str(), nullptr, MS_REMOUNT | MS_BIND, ""), + EPERM); + // The mount is still read-only. + TEST_CHECK_ERRNO(open(file_path.c_str(), O_CREAT | O_RDWR, 0666), EROFS); + // A remount that keeps all of the locked flags set is allowed. + TEST_CHECK_SUCCESS(mount("", path.c_str(), nullptr, + MS_REMOUNT | MS_BIND | kLockableFlags, "")); + }; + EXPECT_THAT(InForkedUserMountNamespace([] {}, child), + IsPosixErrorOkAndHolds(0)); + + // The flags are not locked in the namespace that owns the mount. + EXPECT_THAT(mount("", path.c_str(), nullptr, MS_REMOUNT | MS_BIND, ""), + SyscallSucceeds()); + struct statfs st; + ASSERT_THAT(statfs(path.c_str(), &st), SyscallSucceeds()); + EXPECT_EQ(st.f_flags & kLockableFlags, 0); +} + +TEST(MountTest, LockedMountFlagsAreInheritedByBind) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + const Cleanup mount_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), kTmpfs, 0, "", 0)); + ASSERT_NO_FATAL_FAILURE(SetLockableFlags(dir.path())); + const TempPath target = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + const std::string path = dir.path(); + const std::string target_path = target.path(); + + const std::function child = [&] { + // Binding the locked mount elsewhere must not result in a mount for which + // flags can be cleared. + TEST_CHECK_SUCCESS( + mount(path.c_str(), target_path.c_str(), "", MS_BIND, "")); + for (const int flag : {MS_RDONLY, MS_NOEXEC, MS_NODEV, MS_NOSUID}) { + TEST_CHECK_ERRNO(mount("", target_path.c_str(), nullptr, + MS_REMOUNT | MS_BIND | (kLockableFlags & ~flag), + ""), + EPERM); + } + TEST_CHECK_SUCCESS(mount("", target_path.c_str(), nullptr, + MS_REMOUNT | MS_BIND | kLockableFlags, "")); + // The bind mount itself was created in this namespace, so unlike the mount + // it was cloned from, it can be unmounted. + TEST_CHECK_SUCCESS(umount2(target_path.c_str(), MNT_DETACH)); + }; + EXPECT_THAT(InForkedUserMountNamespace([] {}, child), + IsPosixErrorOkAndHolds(0)); +} + TEST(MountTest, RemountUnmounted) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));