[Devel] [PATCH vz10] fs: let the protection flags of a mount be locked from userspace
Mirian Shilakadze
mirian.shilakadze at virtuozzo.com
Mon Oct 12 13:20:34 MSK 2026
A container can strip ro, nosuid, nodev or noexec from a bind mount the
host gave it with "mount -o remount,rw,bind" and then write host data.
Nothing raises the MNT_LOCK_* bits that forbid this on such a mount, and
userspace cannot set them.
Add MOUNT_ATTR_VE_LOCK to mount_setattr(). In attr_set it locks those
of the four protections the call leaves set, in attr_clr it drops the
locks. Only a host task with CAP_SYS_ADMIN in the initial user
namespace may use it. MNT_LOCKED is not raised, so the container can
still unmount the bind.
Locks can now appear on an attached mount, so can_change_locked_flags()
is re-tested under the mount hash in do_reconfigure_mnt() and
do_remount().
Feature: ve: ve generic structures
https://virtuozzo.atlassian.net/browse/VSTOR-141092
Signed-off-by: Mirian Shilakadze <mirian.shilakadze at virtuozzo.com>
---
The libvzctl side, which locks bindmounts at container start, on a live
add and again after restore, goes to vzctl.ovz separately.
Tested on 211.52.1.22.8 in a VM:
- a container cannot strip ro, nosuid, nodev or noexec from a locked bind
by remount or mount_setattr(), cannot unlock it and cannot lock its own
mounts, a bind it makes of a locked mount inherits the lock
- an unprivileged user, root without CAP_SYS_ADMIN and root in a user
namespace of its own cannot unlock what lock_mnt_tree() locked
- a VE in pseudosuper cannot lock or unlock, the host can do both
- mount_setattr selftest 21/21, mount/ selftests pass
- a remount racing a lock, with a 2 ms sleep added to widen the window:
about 30% of rounds end writable but locked without the re-tests, none
in 26000 rounds with them
fs/namespace.c | 113 ++++++++++++++----
include/uapi/linux/mount.h | 22 ++++
.../trace/beauty/include/uapi/linux/mount.h | 22 ++++
3 files changed, 137 insertions(+), 20 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 75cabf7763ef..07ee860cae6e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -100,6 +100,7 @@ struct mount_kattr {
unsigned int propagation;
unsigned int lookup_flags;
bool recurse;
+ bool ve_lock;
struct user_namespace *mnt_userns;
struct mnt_idmap *mnt_idmap;
};
@@ -2394,26 +2395,42 @@ int iterate_mounts(int (*f)(struct vfsmount *, void *), void *arg,
return 0;
}
+#define MNT_LOCK_PROTECTION_MASK (MNT_LOCK_READONLY | MNT_LOCK_NOSUID | \
+ MNT_LOCK_NODEV | MNT_LOCK_NOEXEC)
+
+/*
+ * Raise the MNT_LOCK_* twin of every protection @flags carries, so that whoever
+ * is given the mount cannot clear them again. Shared by lock_mnt_tree(), where
+ * the kernel locks a tree that crosses a user namespace, and by
+ * MOUNT_ATTR_VE_LOCK, where the host asks for it.
+ *
+ * Neither MNT_LOCKED nor MNT_LOCK_ATIME is raised here. lock_mnt_tree() adds
+ * both itself, while a mount locked through MOUNT_ATTR_VE_LOCK can still be
+ * unmounted and keeps its atime settable.
+ */
+static unsigned int lock_protection_flags(unsigned int flags)
+{
+ if (flags & MNT_READONLY)
+ flags |= MNT_LOCK_READONLY;
+ if (flags & MNT_NOSUID)
+ flags |= MNT_LOCK_NOSUID;
+ if (flags & MNT_NODEV)
+ flags |= MNT_LOCK_NODEV;
+ if (flags & MNT_NOEXEC)
+ flags |= MNT_LOCK_NOEXEC;
+
+ return flags;
+}
+
static void lock_mnt_tree(struct mount *mnt)
{
struct mount *p;
for (p = mnt; p; p = next_mnt(p, mnt)) {
- int flags = p->mnt.mnt_flags;
+ int flags = lock_protection_flags(p->mnt.mnt_flags);
+
/* Don't allow unprivileged users to change mount flags */
flags |= MNT_LOCK_ATIME;
-
- if (flags & MNT_READONLY)
- flags |= MNT_LOCK_READONLY;
-
- if (flags & MNT_NODEV)
- flags |= MNT_LOCK_NODEV;
-
- if (flags & MNT_NOSUID)
- flags |= MNT_LOCK_NOSUID;
-
- if (flags & MNT_NOEXEC)
- flags |= MNT_LOCK_NOEXEC;
/* Don't allow unprivileged users to reveal what is under a mount */
if (list_empty(&p->mnt_expire))
flags |= MNT_LOCKED;
@@ -3018,8 +3035,10 @@ SYSCALL_DEFINE3(open_tree, int, dfd, const char __user *, filename, unsigned, fl
/*
* Don't allow locked mount flags to be cleared.
*
- * No locks need to be held here while testing the various MNT_LOCK
- * flags because those flags can never be cleared once they are set.
+ * MOUNT_ATTR_VE_LOCK can raise and drop the MNT_LOCK_* flags on a mount that
+ * is already attached, so a test made without the mount hash held only tells
+ * you what was true a moment ago. Test again under the hash before the change
+ * that depends on it.
*/
static bool can_change_locked_flags(struct mount *mnt, unsigned int mnt_flags)
{
@@ -3124,9 +3143,12 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
*/
down_read(&sb->s_umount);
lock_mount_hash();
- ret = change_mount_ro_state(mnt, mnt_flags);
- if (ret == 0)
- set_mount_attributes(mnt, mnt_flags);
+ ret = -EPERM;
+ if (can_change_locked_flags(mnt, mnt_flags)) {
+ ret = change_mount_ro_state(mnt, mnt_flags);
+ if (ret == 0)
+ set_mount_attributes(mnt, mnt_flags);
+ }
unlock_mount_hash();
up_read(&sb->s_umount);
@@ -3382,6 +3404,20 @@ static inline int ve_try_reserve_mount(struct ve_struct *ve)
return ret;
}
+/*
+ * Only the host may lock or unlock the protection flags of a mount, meaning a
+ * task in ve0 with CAP_SYS_ADMIN in the initial user namespace.
+ *
+ * ve0 alone proves nothing. Every host task is there, an unprivileged user in a
+ * user namespace of its own included, and could otherwise unlock what
+ * lock_mnt_tree() locked against it. A VE in pseudosuper is not enough either,
+ * libvzctl runs container binaries inside it while the flag is still raised.
+ */
+static inline bool ve_may_lock_mount(void)
+{
+ return ve_is_super(get_exec_env()) && capable(CAP_SYS_ADMIN);
+}
+
static inline void ve_mount_put(struct mount *mnt, struct ve_struct *ve)
{
/*
@@ -3440,6 +3476,7 @@ bool is_sb_ve_accessible(struct ve_struct *ve, struct super_block *sb)
static inline int ve_try_reserve_mount(struct ve_struct *ve) { return 1; }
static inline void ve_mount_put(struct mount *mnt, struct ve_struct *ve) { }
static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve) { }
+static inline bool ve_may_lock_mount(void) { return capable(CAP_SYS_ADMIN); }
#endif /* CONFIG_VE */
@@ -3483,7 +3520,15 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
err = reconfigure_super(fc);
if (!err) {
lock_mount_hash();
- set_mount_attributes(mnt, mnt_flags);
+ /*
+ * A lock may have landed since the first test.
+ * The superblock change above stands, the mount
+ * flags do not change.
+ */
+ if (can_change_locked_flags(mnt, mnt_flags))
+ set_mount_attributes(mnt, mnt_flags);
+ else
+ err = -EPERM;
unlock_mount_hash();
}
}
@@ -4543,7 +4588,8 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
MOUNT_ATTR_NOEXEC | MOUNT_ATTR__ATIME | MOUNT_ATTR_NODIRATIME | \
MOUNT_ATTR_NOSYMFOLLOW)
-#define MOUNT_SETATTR_VALID_FLAGS (FSMOUNT_VALID_FLAGS | MOUNT_ATTR_IDMAP)
+#define MOUNT_SETATTR_VALID_FLAGS \
+ (FSMOUNT_VALID_FLAGS | MOUNT_ATTR_IDMAP | MOUNT_ATTR_VE_LOCK)
#define MOUNT_SETATTR_PROPAGATION_FLAGS \
(MS_UNBINDABLE | MS_PRIVATE | MS_SLAVE | MS_SHARED)
@@ -5052,6 +5098,8 @@ static void mount_setattr_commit(struct mount_kattr *kattr, struct mount *mnt)
do_idmap_mount(kattr, m);
flags = recalc_flags(kattr, m);
+ if (kattr->ve_lock)
+ flags = lock_protection_flags(flags);
WRITE_ONCE(m->mnt.mnt_flags, flags);
/* If we had to hold writers unblock them. */
@@ -5230,8 +5278,33 @@ static int build_mount_kattr(const struct mount_attr *attr, size_t usize,
if ((attr->attr_set | attr->attr_clr) & ~MOUNT_SETATTR_VALID_FLAGS)
return -EINVAL;
+ kattr->ve_lock = !!(attr->attr_set & MOUNT_ATTR_VE_LOCK);
+ if (kattr->ve_lock || (attr->attr_clr & MOUNT_ATTR_VE_LOCK)) {
+ if (kattr->ve_lock && (attr->attr_clr & MOUNT_ATTR_VE_LOCK))
+ return -EINVAL;
+ /*
+ * Unlocking says nothing about the protections themselves.
+ * Relaxing one is a separate call, so it still goes through
+ * can_change_locked_flags() against a mount that is by then
+ * genuinely unlocked, rather than riding in on this one.
+ */
+ if ((attr->attr_clr & MOUNT_ATTR_VE_LOCK) &&
+ (attr->attr_clr & (MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOSUID |
+ MOUNT_ATTR_NODEV | MOUNT_ATTR_NOEXEC)))
+ return -EINVAL;
+ if (!ve_may_lock_mount())
+ return -EPERM;
+ }
+
kattr->attr_set = attr_flags_to_mnt_flags(attr->attr_set);
kattr->attr_clr = attr_flags_to_mnt_flags(attr->attr_clr);
+ /*
+ * Unlocking is a plain clear, so recalc_flags() is already all it
+ * needs. Locking cannot be: which twin to raise depends on what each
+ * mount in the tree ends up with, which is only known per mount.
+ */
+ if (attr->attr_clr & MOUNT_ATTR_VE_LOCK)
+ kattr->attr_clr |= MNT_LOCK_PROTECTION_MASK;
/*
* Since the MOUNT_ATTR_<atime> values are an enum, not a bitmap,
diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h
index 225bc366ffcb..6368876b37ad 100644
--- a/include/uapi/linux/mount.h
+++ b/include/uapi/linux/mount.h
@@ -125,6 +125,28 @@ enum fsconfig_command {
#define MOUNT_ATTR_IDMAP 0x00100000 /* Idmap mount to @userns_fd in struct mount_attr. */
#define MOUNT_ATTR_NOSYMFOLLOW 0x00200000 /* Do not follow symlinks */
+/*
+ * Virtuozzo mount attributes use high bits (from 63 downwards) to stay clear of
+ * the upstream range, which allocates upwards from bit 0. Keep them last in this
+ * list, after the mainline entries above, so a future upstream flag creeping up
+ * towards them is easy to spot.
+ */
+
+/*
+ * In attr_set, lock MOUNT_ATTR_RDONLY, MOUNT_ATTR_NOSUID, MOUNT_ATTR_NODEV and
+ * MOUNT_ATTR_NOEXEC, whichever of them the call leaves set, so that whoever is
+ * given the mount cannot clear them again. Those four are the only attributes
+ * this flag locks. In attr_clr, drop those locks again, on its own and not
+ * alongside the attributes it was guarding. mount_setattr() only, fsmount()
+ * takes its attributes as 32 bits and never sees this one.
+ *
+ * Only the host may do either, so a container cannot unlock what it was given
+ * and cannot lock anything against the host. The lock is per mount and is
+ * copied only into binds made after it, so set it before the mount is handed
+ * over, on the detached tree.
+ */
+#define MOUNT_ATTR_VE_LOCK 0x8000000000000000ULL
+
/*
* mount_setattr()
*/
diff --git a/tools/perf/trace/beauty/include/uapi/linux/mount.h b/tools/perf/trace/beauty/include/uapi/linux/mount.h
index 225bc366ffcb..6368876b37ad 100644
--- a/tools/perf/trace/beauty/include/uapi/linux/mount.h
+++ b/tools/perf/trace/beauty/include/uapi/linux/mount.h
@@ -125,6 +125,28 @@ enum fsconfig_command {
#define MOUNT_ATTR_IDMAP 0x00100000 /* Idmap mount to @userns_fd in struct mount_attr. */
#define MOUNT_ATTR_NOSYMFOLLOW 0x00200000 /* Do not follow symlinks */
+/*
+ * Virtuozzo mount attributes use high bits (from 63 downwards) to stay clear of
+ * the upstream range, which allocates upwards from bit 0. Keep them last in this
+ * list, after the mainline entries above, so a future upstream flag creeping up
+ * towards them is easy to spot.
+ */
+
+/*
+ * In attr_set, lock MOUNT_ATTR_RDONLY, MOUNT_ATTR_NOSUID, MOUNT_ATTR_NODEV and
+ * MOUNT_ATTR_NOEXEC, whichever of them the call leaves set, so that whoever is
+ * given the mount cannot clear them again. Those four are the only attributes
+ * this flag locks. In attr_clr, drop those locks again, on its own and not
+ * alongside the attributes it was guarding. mount_setattr() only, fsmount()
+ * takes its attributes as 32 bits and never sees this one.
+ *
+ * Only the host may do either, so a container cannot unlock what it was given
+ * and cannot lock anything against the host. The lock is per mount and is
+ * copied only into binds made after it, so set it before the mount is handed
+ * over, on the detached tree.
+ */
+#define MOUNT_ATTR_VE_LOCK 0x8000000000000000ULL
+
/*
* mount_setattr()
*/
--
2.43.0
More information about the Devel
mailing list