[Devel] [PATCH v7 VZ10] fs: enforce container device-mount policy in the common mount path
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Fri Jul 31 17:26:22 MSK 2026
On 7/31/26 16:00, Pavel Tikhomirov wrote:
> Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>
> On 7/23/26 15:49, Vasileios Almpanis wrote:
>> In a container the per-device ve_devmnt policy restricts which options
>> a device may be mounted with and force-inserts a set of hidden options.
>> The check used to run inside the option-string parser
>> (vfs_parse_monolithic_sep) and, for remount, in a separate helper. Two
>> things escaped it:
>>
>> - MS_* flags from the legacy mount(2)/fsconfig(2) API are folded into
>> fc->sb_flags and never appear in the option string, so a container
>> could set MS_RDONLY, MS_SYNCHRONOUS, MS_MANDLOCK, ... outside its
>> allowed set.
>>
>> - The check sat in filesystem-selectable callbacks (->parse_monolithic,
>> ->mount), so a filesystem not routing through them evaded the policy,
>> and a skipped hidden-option insertion dropped a container's mandated
>> options without error.
>>
>> Enforce the policy in the fs-agnostic common mount path instead:
>> vfs_get_tree() for a new mount and reconfigure_super() for a remount.
>> The device is taken from the mounted superblock, so fc->source cannot be
>> raced to target another device, and fc->sb_flags is vetted alongside the
>> option string. On a new mount the forced options must also be present,
>> so a filesystem that skipped inserting them has its mount refused rather
>> than silently losing them.
>>
>> The parse-time ve_devmnt_process() call is kept as a best-effort early
>> reject, so a disallowed device or option is refused before the
>> filesystem's fill_super() runs.
>>
>> Fix a bug where a containers that use the legacy mount(2) syscall are able
>> to reconfigure a mount and change the superblock flags, for example
>> from RO to RW. Compute the effective superblock flags and emit rw incase
>> SB_RDONLY is missing in vfs_format_sb_flags.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-132330
>> Fixes: 263467c864c5 ("ve/fs/devmnt: process mount options")
>> Signed-off-by: Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>
>> Co-developed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>>
>> Feature: ve: ve generic structures
>> ---
>> Changes since v6:
>> - Enforce sb-flag changes on remount: emit "rw" when SB_RDONLY is cleared
>> and vet the effective flags,
>> (sb->s_flags & ~fc->sb_flags_mask) | (fc->sb_flags & fc->sb_flags_mask),
>> so a legacy mount(2) remount can no longer flip RO->RW unchecked.
>> - Constrain only devices listed in ve.mount_opts: an unlisted device
>> mounted with no userspace options is allowed rather than refused by the
>> synthesized ro/rw token (tracked via have_user_opts).
>> - Reset fc->ve_final_opts in vfs_dup_fs_context() to avoid a double-free.
>> - Factor per-option emit into __vfs_emit_flag(); checkpatch/style fixes.
>>
>> Changes since v5:
>> - Fix use-after-free: do_new_mount() freed the merged page that
>> legacy_get_tree()->mount() dereferences later; the fs now sees only
>> the caller-owned original data.
>> - Fix remount,dirsync failing with -EINVAL in containers: stop re-parsing
>> the formatted flag names (SB_DIRSYNC is not in MS_RMT_MASK).
>> - Check policy only: legacy fs parsers no longer get synthesized ro/sync
>> tokens, and the flag check reads fc->sb_flags symmetrically for mount
>> and fsconfig.
>> - Keep ve_check_mount_options() only on the remount path (pinned sb
>> device, covers legacy ->reconfigure); drop legacy_merge_mount_data()
>> and ve_prepare_mount_options(); add Fixes: tag and rewrite the message.
>>
>> Changes since v4:
>> - Emit only the positive sb-flag names, not the clear names (rw/async/
>> ...); on legacy remount sb_flags_mask is MS_RMT_MASK, so the clear
>> names had wrongly rejected ordinary in-container remounts.
>> - NUL-terminate the options page when data is empty and no flags emitted.
>> - Fix __vfs_format_flags() comment (-E2BIG, not -ENOSPC).
>>
>> Changes since v3:
>> - Drop excess length check in legacy_merge_mount_data().
>>
>> Changes since v2:
>> - Remove the legacy_merge_mount_data guard in fs/internal.h.
>> - Add __vfs_format_flags() helper, used by vfs_format_sb_flags().
>> - Use -E2BIG (not -ENOSPC) for the buffer-too-small case.
>>
>> Changes since v1:
>> - Unify the comma-insert-copy pattern across call sites via an
>> append_entry() helper.
>> - Rework legacy_merge_mount_data() to allocate the page upfront and
>> append sb flags via vfs_format_sb_flags(), dropping flags_buf[128]
>> and the size arithmetic.
>> - Use -ENOSPC (not -EINVAL) for buffer-too-small; comment
>> FS_BINARY_MOUNTDATA; minor blank-line cleanups.
>>
>> fs/fs_context.c | 159 +++++++++++++++++++++++++++++++++++++
>> fs/internal.h | 1 +
>> fs/namespace.c | 110 +++++++++++++++++++------
>> fs/super.c | 12 +++
>> include/linux/fs_context.h | 2 +
>> include/linux/mount.h | 2 +
>> 6 files changed, 263 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/fs_context.c b/fs/fs_context.c
>> index 76f34f3d468e..8cf06e2ac800 100644
>> --- a/fs/fs_context.c
>> +++ b/fs/fs_context.c
>> @@ -81,6 +81,70 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
>> return -ENOPARAM;
>> }
>>
>> +/*
>> + * Emit option @name into @buff at *@off, prefixed with ',' if the buffer
>> + * already holds text. Advances *@off. Returns 0 or -E2BIG if @buff is full.
>> + */
>> +static int __vfs_emit_flag(const char *name, char *buff,
>> + size_t size, size_t *off)
>> +{
>> + ssize_t ret;
>> +
>> + if (*off) {
>> + if (*off + 1 >= size)
>> + return -E2BIG;
>> + buff[(*off)++] = ',';
>> + }
>> +
>> + ret = strscpy(buff + *off, name, size - *off);
>> + if (ret < 0)
>> + return -E2BIG;
>> + *off += ret;
>> + return 0;
>> +}
>> +
>> +static int __vfs_format_flags(const struct constant_table *p, unsigned int flags,
>> + char *buff, size_t size, size_t *off)
>> +{
>> + for (; p->name; p++) {
>> + int ret;
>> +
>> + if (!(flags & p->value))
>> + continue;
>> + ret = __vfs_emit_flag(p->name, buff, size, off);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> +static int vfs_format_sb_flags(char *buff, size_t size, size_t *off,
>> + unsigned int sb_flags)
>> +{
>> + int err;
>> +
>> + err = __vfs_format_flags(common_set_sb_flag, sb_flags, buff, size, off);
>> + if (err)
>> + return err;
>> +
>> + /*
>> + * "rw" has no flag bit of its own - it is simply the absence of
>> + * SB_RDONLY. Emit it explicitly so the ve_devmnt policy can allow or
>> + * deny read-write access as a first-class option; otherwise a mount or
>> + * remount that leaves the superblock read-write carries no token and
>> + * slips past the "every option must be allowed" check.
>> + *
>> + * @sb_flags is the effective post-operation flag word, so this reflects
>> + * the state the superblock actually ends up in. A remount that only
>> + * touches an unrelated flag (e.g. "sync") keeps its current SB_RDONLY
>> + * and so does not emit "rw".
>> + */
>> + if (!(sb_flags & SB_RDONLY))
>> + return __vfs_emit_flag("rw", buff, size, off);
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * vfs_parse_fs_param_source - Handle setting "source" via parameter
>> * @fc: The filesystem context to modify
>> @@ -224,6 +288,85 @@ static inline int fscontext_lookup_bdev(struct fs_context *fc, dev_t *s_dev)
>> return -ENODEV;
>> }
>>
>> +/*
>> + * ve_devmnt_verify_fc - check a mount against the container device-mount policy
>> + * @fc: the mount context, with fc->root set
>> + * @new_mount: true at vfs_get_tree() (new mount), false at reconfigure_super()
>> + *
>> + * Vets the stashed userspace option string plus the synthesized SB_* flag
>> + * names against the mounted superblock's device. A device absent from the
>> + * policy that is mounted with no userspace options is allowed. Returns 0 when
>> + * permitted (or no check applies), or a negative errno.
>> + */
>> +int ve_devmnt_verify_fc(struct fs_context *fc, bool new_mount)
>> +{
>> + struct ve_struct *ve = get_exec_env();
>> + unsigned int sb_flags;
>> + bool have_user_opts;
>> + size_t off = 0;
>> + char *page;
>> + int err;
>> +
>> + if (ve_is_super(ve))
>> + return 0;
>> +
>> + if (!fc->fs_type || !(fc->fs_type->fs_flags & FS_REQUIRES_DEV))
>> + return 0;
>> +
>> + /*
>> + * Filesystems with binary mount data (e.g. btrfs) bypass option
>> + * string parsing entirely, so our checks cannot apply here.
>> + */
>> + if (fc->fs_type->fs_flags & FS_BINARY_MOUNTDATA)
>> + return 0;
>> +
>> + if (WARN_ON_ONCE(!fc->root))
>> + return -EINVAL;
>> +
>> + page = (char *)__get_free_page(GFP_KERNEL);
>> + if (!page)
>> + return -ENOMEM;
>> +
>> + /*
>> + * Track whether userspace actually supplied options. @page below also
>> + * gets the synthesized ro/rw flag token, so its length cannot answer
>> + * this; ve_final_opts holds only the userspace string.
>> + */
>> + have_user_opts = fc->ve_final_opts && *fc->ve_final_opts;
>> + if (have_user_opts) {
>> + ssize_t ret = strscpy(page, fc->ve_final_opts, PAGE_SIZE);
>> +
>> + if (ret < 0) {
>> + err = -E2BIG;
>> + goto out;
>> + }
>> + off = ret;
>> + }
>> +
>> + /*
>> + * On a remount fc->sb_flags holds only the bits being changed, so
>> + * combine them with the current superblock flags to get the state the
>> + * sb will actually have - the same value reconfigure_super() writes
>> + * back. On a new mount fc->sb_flags is already the full flag word.
>> + */
>> + sb_flags = fc->sb_flags;
>> + if (!new_mount)
>> + sb_flags = (fc->root->d_sb->s_flags & ~fc->sb_flags_mask) |
>> + (fc->sb_flags & fc->sb_flags_mask);
>> +
>> + err = vfs_format_sb_flags(page, PAGE_SIZE, &off, sb_flags);
>> + if (err)
>> + goto out;
>> +
>> + page[off] = '\0';
>> + err = ve_devmnt_verify(ve, fc->root->d_sb->s_dev, page, new_mount,
>> + have_user_opts);
>> +
>> +out:
>> + free_page((unsigned long)page);
>> + return err;
>> +}
>> +
>> static int fscontext_init_lazy_opts(struct fs_context *fc)
>> {
>> struct ve_struct *ve = get_exec_env();
>> @@ -389,10 +532,22 @@ int vfs_parse_monolithic_sep(struct fs_context *fc, void *data,
>> return -ENODEV;
>> }
>>
>> + /* Early reject and hidden-option insertion; verified for real later. */
>> ret = ve_devmnt_process(ve, bd_dev, (void **) &options,
>> fc->purpose == FS_CONTEXT_FOR_RECONFIGURE);
>> if (ret)
>> return ret;
>> +
>> + /* Stash what the filesystem parses; checked in the common mount path. */
>> + if (options) {
>> + kfree(fc->ve_final_opts);
>> + fc->ve_final_opts = kstrdup(options, GFP_KERNEL);
>> + if (!fc->ve_final_opts) {
>> + if (options != options_orig)
>> + free_page((unsigned long)options);
>> + return -ENOMEM;
>> + }
>> + }
>> }
>>
>> /*
>> @@ -614,6 +769,7 @@ struct fs_context *vfs_dup_fs_context(struct fs_context *src_fc)
>> fc->s_fs_info = NULL;
>> fc->source = NULL;
>> fc->security = NULL;
>> + fc->ve_final_opts = NULL;
>> get_filesystem(fc->fs_type);
>> get_net(fc->net_ns);
>> get_user_ns(fc->user_ns);
>> @@ -742,6 +898,7 @@ void put_fs_context(struct fs_context *fc)
>> put_filesystem(fc->fs_type);
>> if (fc->lazy_opts)
>> free_page((unsigned long)fc->lazy_opts);
>> + kfree(fc->ve_final_opts);
>> kfree(fc->source);
>> kfree(fc);
>> }
>> @@ -962,6 +1119,8 @@ void vfs_clean_context(struct fs_context *fc)
>> free_page((unsigned long)fc->lazy_opts);
>> fc->lazy_opts = NULL;
>> }
>> + kfree(fc->ve_final_opts);
>> + fc->ve_final_opts = NULL;
>> kfree(fc->source);
>> fc->source = NULL;
>> fc->exclusive = false;
>> diff --git a/fs/internal.h b/fs/internal.h
>> index 3647ce69b2c7..e33d3ae70cce 100644
>> --- a/fs/internal.h
>> +++ b/fs/internal.h
>> @@ -46,6 +46,7 @@ extern void __init chrdev_init(void);
>> */
>> extern const struct fs_context_operations legacy_fs_context_ops;
>> extern int parse_monolithic_mount_data(struct fs_context *, void *);
>> +extern int ve_devmnt_verify_fc(struct fs_context *fc, bool new_mount);
>> extern void vfs_clean_context(struct fs_context *fc);
>> extern int finish_clean_context(struct fs_context *fc);
>>
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 0f4a3668e558..0b46158852e6 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -3258,6 +3258,92 @@ int ve_devmnt_process(struct ve_struct *ve, dev_t dev, void **data_pp, int remou
>> return err;
>> }
>>
>> +/* Return 0 if every option in @options is listed in @a or @b, else -EPERM. */
>> +static int ve_devmnt_options_subset(char *options, char *a, char *b)
>> +{
>> + char *copy, *cur, *p;
>> + int err = 0;
>> +
>> + if (!options || !*options)
>> + return 0;
>> + if (!a && !b)
>> + return -EPERM;
>> +
>> + copy = cur = kstrdup(options, GFP_KERNEL);
>> + if (!copy)
>> + return -ENOMEM;
>> +
>> + while ((p = strsep(&cur, ",")) != NULL) {
>> + if (!*p)
>> + continue;
>> + if ((!a || !strstr_separated(a, p, ',')) &&
>> + (!b || !strstr_separated(b, p, ','))) {
>> + err = -EPERM;
>> + break;
>> + }
>> + }
>> +
>> + kfree(copy);
>> + return err;
>> +}
>> +
>> +/*
>> + * ve_devmnt_verify - enforce the container device-mount policy for @dev
>> + * @ve: the container
>> + * @dev: device taken from the mounted superblock (not from a raceable path)
>> + * @opts: mount options plus the SB_* flag names to vet
>> + * @new_mount: true for a new mount, false for a remount
>> + * @have_user_opts: true if userspace supplied any mount options. @opts always
>> + * carries the kernel-synthesized ro/rw flag token, so it is never empty
>> + * and cannot answer this on its own.
>> + *
>> + * Every supplied option must be allowed or forced. On a new mount the forced
>> + * ("hidden") options must also be present: a filesystem that skipped inserting
>> + * them is refused rather than silently dropping a container's mandated option
>> + */
>> +int ve_devmnt_verify(struct ve_struct *ve, dev_t dev, char *opts, bool new_mount,
>> + bool have_user_opts)
>> +{
>> + struct ve_devmnt *devmnt;
>> + char *allowed = NULL, *hidden = NULL;
>> + bool found = false;
>> + int err = 0;
>> +
>> + if (ve->is_pseudosuper)
>> + return 0;
>> +
>> + mutex_lock(&ve->devmnt_mutex);
>> + list_for_each_entry(devmnt, &ve->devmnt_list, link) {
>> + if (devmnt->dev == dev) {
>> + allowed = devmnt->allowed_options;
>> + hidden = devmnt->hidden_options;
>> + found = true;
>> + break;
>> + }
>> + }
>> +
>> + /*
>> + * Enforce for a listed device, or for any mount carrying userspace
>> + * options. An unlisted device with no userspace options is unconstrained
>> + * here, so the synthesized ro/rw token in @opts does not deny it.
>> + */
>> + if (found || have_user_opts) {
On the second thought:
Thе whole point of ve_devmnt_verify is to ALWAYS make sure that all options
are vaild. This if is suspicious.
1) If we've found an entry for device we apply full restrictions. -> good case.
2) If fs filled fc->ve_final_opts with some options we check them. -> good case.
3) If fs does not fill fc->ve_final_opts we don't know the options, we can't check them, we should refuse the mount. But we don't. -> bad case.
4) If user provided empty options, we still should check them. (hidden shoudl still be appended, empty is subset of hidden and found so everything is allowed)
5) "rw" is really a default option, it should not be checked against allowed, instead I believe if we want to prevent it we should add "ro" to hidden instead, and check that "ro" is present at this point.
>> + /* every supplied option must be either allowed or forced */
>> + err = ve_devmnt_options_subset(opts, allowed, hidden);
>> +
>> + /* on a new mount every forced option must have reached the fs */
>> + if (!err && new_mount)
>> + err = ve_devmnt_options_subset(hidden, opts, NULL);
>> + }
>> + mutex_unlock(&ve->devmnt_mutex);
>> +
>> + if (err == -EPERM)
>> + ve_pr_warn_ratelimited(VE_LOG_BOTH,
>> + "VE%s: mount options not permitted for device %u:%u\n",
>> + ve_name(ve), MAJOR(dev), MINOR(dev));
>> + return err;
>> +}
>> +
>> static inline int ve_mount_allowed(void)
>> {
>> struct ve_struct *ve = get_exec_env();
>> @@ -3308,23 +3394,6 @@ static inline void ve_mount_nr_inc(struct mount *mnt, struct ve_struct *ve) { }
>> static inline void ve_mount_nr_dec(struct mount *mnt) { }
>> #endif /* CONFIG_VE */
>>
>> -static int ve_prepare_mount_options(struct fs_context *fc, void *data)
>> -{
>> -#ifdef CONFIG_VE
>> - struct super_block *sb = fc->root->d_sb;
>> - struct ve_struct *ve = get_exec_env();
>> -
>> - if (sb->s_bdev && data && !ve_is_super(ve)) {
>> - int err;
>> -
>> - err = ve_devmnt_process(ve, sb->s_bdev->bd_dev, &data, 1);
>> - if (err)
>> - return err;
>> - }
>> -#endif
>> - return 0;
>> -}
>> -
>> /*
>> * change filesystem flags. dir should be a physical root of filesystem.
>> * If you've mounted a non-root directory somewhere and want to do remount
>> @@ -3357,12 +3426,6 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
>> */
>> fc->oldapi = true;
>>
>> - err = ve_prepare_mount_options(fc, data);
>> - if (err) {
>> - put_fs_context(fc);
>> - return err;
>> - }
>> -
>> err = parse_monolithic_mount_data(fc, data);
>> if (!err) {
>> down_write(&sb->s_umount);
>> @@ -3816,6 +3879,7 @@ static int do_new_mount(struct path *path, const char *fstype, int sb_flags,
>> subtype, strlen(subtype));
>> if (!err && name)
>> err = vfs_parse_fs_string(fc, "source", name, strlen(name));
>> + /* Container device-mount policy is enforced later, in vfs_get_tree(). */
>> if (!err)
>> err = parse_monolithic_mount_data(fc, data);
>> if (!err && !mount_capable(fc))
>> diff --git a/fs/super.c b/fs/super.c
>> index 1adebbf35803..c0c067eb2d8e 100644
>> --- a/fs/super.c
>> +++ b/fs/super.c
>> @@ -1085,6 +1085,11 @@ int reconfigure_super(struct fs_context *fc)
>> if (retval)
>> return retval;
>>
>> + /* Enforce the container device-mount policy on the remount options. */
>> + retval = ve_devmnt_verify_fc(fc, false);
>> + if (retval)
>> + return retval;
>> +
>> if (fc->sb_flags_mask & SB_RDONLY) {
>> #ifdef CONFIG_BLOCK
>> if (!(fc->sb_flags & SB_RDONLY) && sb->s_bdev &&
>> @@ -1924,6 +1929,13 @@ int vfs_get_tree(struct fs_context *fc)
>> return error;
>> }
>>
>> + /* Enforce the container device-mount policy against the real device. */
>> + error = ve_devmnt_verify_fc(fc, true);
>> + if (unlikely(error)) {
>> + fc_drop_locked(fc);
>> + return error;
>> + }
>> +
>> /*
>> * filesystems should never set s_maxbytes larger than MAX_LFS_FILESIZE
>> * but s_maxbytes was an unsigned long long for many releases. Throw
>> diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
>> index 1801aed1da67..2ca586e2cc2a 100644
>> --- a/include/linux/fs_context.h
>> +++ b/include/linux/fs_context.h
>> @@ -93,6 +93,8 @@ struct fs_context {
>> struct file_system_type *fs_type;
>> void *fs_private; /* The filesystem's context */
>> void *lazy_opts; /* mount options which can't be checked at fsconfig() time */
>> + /* option string handed to the fs, for the ve_devmnt policy check */
>> + char *ve_final_opts;
>> void *sget_key;
>> struct dentry *root; /* The root and superblock */
>> struct user_namespace *user_ns; /* The user namespace for this mount */
>> diff --git a/include/linux/mount.h b/include/linux/mount.h
>> index 0cbc6f6893c0..ab0ea4f7afc6 100644
>> --- a/include/linux/mount.h
>> +++ b/include/linux/mount.h
>> @@ -127,5 +127,7 @@ extern int cifs_root_data(char **dev, char **opts);
>>
>> struct ve_struct;
>> extern int ve_devmnt_process(struct ve_struct *, dev_t, void **, int);
>> +extern int ve_devmnt_verify(struct ve_struct *ve, dev_t dev, char *opts,
>> + bool new_mount, bool have_user_opts);
>>
>> #endif /* _LINUX_MOUNT_H */
>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
More information about the Devel
mailing list