[Devel] [PATCH v6 1/1] fs: check MS_* mount flags against container device mount policy
Pavel Tikhomirov
ptikhomirov at virtuozzo.com
Fri Jul 17 15:16:41 MSK 2026
I'm a bit sad that this check for mount options relies on fs-specific callbacks.
I'll send a v6.1 in reply with probably a better way + one more fix. Please consider it.
On 7/7/26 19:25, Vasileios Almpanis wrote:
> In the legacy mount(2) and fsconfig(2) callpaths userspace may pass mount
> options either in the monolithic option string or as MS_* flags. The
> flags are folded into fc->sb_flags and never appear in the option string,
> so they escaped the per-device ve_devmnt policy check in
> ve_devmnt_process(): a container could mount a device with options
> (MS_RDONLY, MS_SYNCHRONOUS, MS_MANDLOCK, ...) that its allowed set does
> not permit.
>
> Reconstruct the names of the set MS_* flags from the existing
> flag -> string tables and run them through the ve_devmnt policy check.
> The check (ve_devmnt_check_sb_flags()) is done in
> vfs_parse_monolithic_sep(), reusing the block device it already resolves
> for the option string, so:
>
> - it applies symmetrically to mount(2) (via parse_monolithic_mount_data)
> and to fsconfig(2) (via the lazy_opts -> generic_parse_monolithic
> finalization in vfs_cmd_create()/vfs_cmd_reconfigure()), because the
> check reads fc->sb_flags regardless of how the flag was supplied;
>
> - the flags are vetted against the very same device resolution used for
> the option string, so a fc->source symlink race cannot get them
> validated against a different device than the one being mounted.
>
> The synthesized flag names are used only for the policy check. They are
> never handed to the filesystem's own option parser (which never saw MS_*
> as strings and would reject them) nor merged into fc->sb_flags_mask
> (which would make reconfigure_super() reject a legacy remount carrying a
> flag outside MS_RMT_MASK, e.g. MS_DIRSYNC).
>
> Legacy filesystems whose ->reconfigure never reaches
> vfs_parse_monolithic_sep() are handled on the remount path by
> ve_check_mount_options(), which runs the same check against the already
> mounted (pinned) superblock device.
>
> 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>
>
> Feature: ve: ve generic structures
> ---
>
> Changes since v5:
> - Fix use-after-free on the legacy mount API: do_new_mount() freed the
> merged page right after parse_monolithic_mount_data(), but
> legacy_parse_monolithic() retains the pointer and legacy_get_tree()
> -> fs_type->mount() dereferences it later. The filesystem now only
> ever sees the caller-owned original data.
> - Fix remount with MS_DIRSYNC failing inside containers: re-parsing the
> formatted "dirsync" set fc->sb_flags_mask |= SB_DIRSYNC, and
> reconfigure_super() then rejected the remount with -EINVAL because
> MS_DIRSYNC is not in MS_RMT_MASK. The flag names are no longer
> re-parsed.
> - Legacy filesystem option parsers no longer receive synthesized
> "ro"/"sync"/... tokens they historically never saw.
> - As a result the flag check is now genuinely symmetric across mount
> and fsconfig: fsconfig reaches vfs_parse_monolithic_sep() via the
> lazy_opts -> generic_parse_monolithic finalization, and the check
> reads fc->sb_flags regardless of how the flag was supplied.
> - Keep ve_check_mount_options() only on the remount path (do_remount),
> where fc->root is set so it uses the pinned superblock device (no
> fc->source re-walk) and covers legacy filesystems whose ->reconfigure
> never reaches vfs_parse_monolithic_sep().
> - Replace legacy_merge_mount_data() (returned a buffer to be parsed)
> with the two check-only helpers above; remove the now-redundant
> ve_prepare_mount_options().
> - Add Fixes: tag; retitle and rewrite the commit message to describe
> the policy-check-only approach.
>
> Changes since v4:
> - Don't emit the negative/clear sb flag names (rw, async, nomand,
> nolazytime) in vfs_format_sb_flags(); emit only the positive names
> (common_set_sb_flag). On the legacy remount path sb_flags_mask is the
> fixed MS_RMT_MASK, so the previous code appended the clear-names for
> every unset remountable bit, and ve_devmnt_check() then rejected
> ordinary remounts within the container not on the host(ve0).
> - Fix uninitialized mount-options page: NUL-terminate the buffer before
> returning it from legacy_merge_mount_data(). When data is empty/NULL
> and no flags are emitted, off stayed 0 and the page from
> __get_free_page() was returned non-terminated.
> - Fix __vfs_format_flags() comment: it returns -E2BIG, not -ENOSPC.
>
> Changes since v3:
> - Drop excess length check in legacy_merge_mount_data
>
> Changes since v2:
> - Remove legacy_merge_mount_data guard in fs/internal.h. All helpers
> don't use anything that would break build and just unchanged pointer
> will be returned.
> - Add __vfs_format_flags helper and use it in vfs_format_sb_flags
> - Fix inconsistent error code: replace -ENOSPC with -E2BIG for the
> buffer-too-small case
>
> Changes since v1:
> - Replace open-coded flag loops with append_entry() helper (pointer-
> advancing style) that unifies the comma-insert-copy pattern across
> all three call sites
> - Rework legacy_merge_mount_data() to allocate the page upfront, write
> user data first then append sb flags via vfs_format_sb_flags(); this
> eliminates the intermediate flags_buf[128], the total size calculation,
> and the +1/+2 arithmetic
> - Fix inconsistent error code: replace -EINVAL with -ENOSPC for the
> buffer-too-small case
> - Add comment on FS_BINARY_MOUNTDATA explaining why those filesystems
> are skipped
> - Add blank line before return in legacy_merge_mount_data()
> - Remove excess blank line after parse_monolithic_mount_data() in
> do_remount()
>
> fs/fs_context.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/internal.h | 1 +
> fs/namespace.c | 33 ++++------
> 3 files changed, 171 insertions(+), 22 deletions(-)
>
> diff --git a/fs/fs_context.c b/fs/fs_context.c
> index 76f34f3d468e..bf4cbc02ef6f 100644
> --- a/fs/fs_context.c
> +++ b/fs/fs_context.c
> @@ -81,6 +81,41 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
> return -ENOPARAM;
> }
>
> +/*
> + * Emit, into @buff at *@off, the comma-separated names of every entry in @p
> + * whose bit is set in @flags. Advances *@off past the written text.
> + * Returns 0 on success or -E2BIG if the buffer is too small.
> + */
> +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++) {
> + ssize_t ret;
> +
> + if (!(flags & p->value))
> + continue;
> +
> + if (*off) {
> + if (*off + 1 >= size)
> + return -E2BIG;
> + buff[(*off)++] = ',';
> + }
> +
> + ret = strscpy(buff + *off, p->name, size - *off);
> + if (ret < 0)
> + return -E2BIG;
> + *off += ret;
> + }
> + return 0;
> +}
> +
> +static int vfs_format_sb_flags(struct fs_context *fc, char *buff, size_t size,
> + size_t *off)
> +{
> + return __vfs_format_flags(common_set_sb_flag, fc->sb_flags,
> + buff, size, off);
> +}
> +
> /**
> * vfs_parse_fs_param_source - Handle setting "source" via parameter
> * @fc: The filesystem context to modify
> @@ -224,6 +259,85 @@ static inline int fscontext_lookup_bdev(struct fs_context *fc, dev_t *s_dev)
> return -ENODEV;
> }
>
> +/*
> + * Legacy remount folds the MS_* mount flags into fc->sb_flags; they never
> + * appear in the monolithic option string. Inside a container the per-device
> + * ve_devmnt policy must still vet them, otherwise a flag such as
> + * MS_RDONLY/MS_SYNCHRONOUS could carry an option the container is not allowed
> + * to set. Build "<user options>,<flag names>" and run it through the policy
> + * check.
> + *
> + * Used from do_remount(): fc->root is already set, so fscontext_lookup_bdev()
> + * resolves to the mounted superblock's device rather than re-walking
> + * fc->source, which both avoids a symlink race and covers legacy filesystems
> + * whose ->reconfigure never reaches vfs_parse_monolithic_sep(). New mounts vet
> + * the flags in vfs_parse_monolithic_sep() instead, sharing the option string's
> + * device resolution.
> + *
> + * Returns 0 when the options are permitted (or no check applies), or a negative
> + * errno otherwise.
> + */
> +int ve_check_mount_options(struct fs_context *fc, void *data)
> +{
> + struct ve_struct *ve = get_exec_env();
> + size_t off = 0;
> + void *options;
> + char *page;
> + dev_t dev;
> + 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;
> +
> + err = fscontext_lookup_bdev(fc, &dev);
> + if (err) {
> + errorf(fc, "%s: Can't lookup blockdev", fc->source);
> + return -ENODEV;
> + }
> +
> + page = (char *)__get_free_page(GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> +
> + if (data && *(char *)data) {
> + ssize_t ret = strscpy(page, data, PAGE_SIZE);
> +
> + if (ret < 0) {
> + err = -E2BIG;
> + goto out;
> + }
> + off = ret;
> + }
> +
> + err = vfs_format_sb_flags(fc, page, PAGE_SIZE, &off);
> + if (err)
> + goto out;
> +
> + page[off] = '\0';
> +
> + /*
> + * Check only: pass remount=1 so ve_devmnt_process() does not insert
> + * hidden options into this throwaway buffer. The real insertion runs
> + * on the original @data in the parse/mount_bdev path.
> + */
> + options = page;
> + err = ve_devmnt_process(ve, dev, &options, 1);
> +
> +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();
> @@ -358,6 +472,40 @@ int vfs_parse_fs_string(struct fs_context *fc, const char *key,
> }
> EXPORT_SYMBOL(vfs_parse_fs_string);
>
> +/*
> + * Vet, against the container's per-device ve_devmnt policy, the names of the
> + * MS_* mount flags folded into fc->sb_flags. Called from
> + * vfs_parse_monolithic_sep() with the @dev it already resolved, so the flags
> + * are checked against the very same device as the option string.
> + *
> + * Check-only (remount=1): the synthesized names are never inserted into the
> + * option string that the filesystem parses.
> + */
> +static int ve_devmnt_check_sb_flags(struct fs_context *fc, struct ve_struct *ve,
> + dev_t dev)
> +{
> + size_t off = 0;
> + void *options;
> + char *page;
> + int err;
> +
> + page = (char *)__get_free_page(GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> +
> + err = vfs_format_sb_flags(fc, page, PAGE_SIZE, &off);
> + if (err || !off) /* error, or no MS_* flags to vet */
> + goto out;
> +
> + page[off] = '\0';
> + options = page;
> + err = ve_devmnt_process(ve, dev, &options, 1);
> +
> +out:
> + free_page((unsigned long)page);
> + return err;
> +}
> +
> /**
> * vfs_parse_monolithic_sep - Parse key[=val][,key[=val]]* mount data
> * @fc: The superblock configuration to fill in.
> @@ -389,6 +537,17 @@ int vfs_parse_monolithic_sep(struct fs_context *fc, void *data,
> return -ENODEV;
> }
>
> + /*
> + * MS_* mount flags from the legacy API live in fc->sb_flags,
> + * not in @options. Vet their names against the same @bd_dev
> + * used for the option string below, so a fc->source symlink
> + * race cannot get them validated against a different device
> + * than the one being mounted.
> + */
> + ret = ve_devmnt_check_sb_flags(fc, ve, bd_dev);
> + if (ret)
> + return ret;
> +
> ret = ve_devmnt_process(ve, bd_dev, (void **) &options,
> fc->purpose == FS_CONTEXT_FOR_RECONFIGURE);
> if (ret)
> diff --git a/fs/internal.h b/fs/internal.h
> index 1b5cb1cda2e4..0871265c111a 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_check_mount_options(struct fs_context *fc, void *data);
> 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 566f11a222fc..1de6a8a61410 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3308,23 +3308,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,11 +3340,9 @@ 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 = ve_check_mount_options(fc, data);
> + if (err)
> + goto out;
>
> err = parse_monolithic_mount_data(fc, data);
> if (!err) {
> @@ -3380,6 +3361,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
>
> mnt_warn_timestamp_expiry(path, &mnt->mnt);
>
> +out:
> put_fs_context(fc);
> return err;
> }
> @@ -3816,6 +3798,13 @@ 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));
> + /*
> + * MS_* mount flags are vetted against the container's ve_devmnt policy
> + * inside parse_monolithic_mount_data() -> vfs_parse_monolithic_sep(),
> + * where they share the block-device resolution used for the option
> + * string. Doing it there (rather than with a separate bdev lookup here)
> + * closes a fc->source symlink race on the flag check.
> + */
> if (!err)
> err = parse_monolithic_mount_data(fc, data);
> if (!err && !mount_capable(fc))
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
More information about the Devel
mailing list