[Devel] [PATCH v6.1 VZ10 1/2] fs: enforce container device-mount policy in the common mount path
Vasileios Almpanis
vasileios.almpanis at virtuozzo.com
Mon Jul 20 11:35:48 MSK 2026
On 7/20/26 12:14 AM, Vladimir Riabchun wrote:
>
>
> On 17.07.2026 14:17, Pavel Tikhomirov wrote:
>> From: Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>
>>
>> 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.
>
> I checked and something seems uncovered: do_reconfigure_mnt via
> MS_REMOUNT|MS_BIND flags doesn't hit vfs_get_tree nor reconfigure_super,
> and yet may update flags.
>
> Am I missing something, maybe we forbid bind mounts somewhere?
Look like you are right. Verified that we can indeed create bind mounts,
in CTs.
We can also bind mount them to another directory and then reconfigure
those bind
mounts with other options (not stated in ve.mount_opts) and it will
succeed. We
could guard bind reconfigures with capable checks otherwise we need get
block device
and go through the same checks. Before I send the next patch I'm open to
suggestions
>> 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.
>>
>> 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
>> ---
>> fs/fs_context.c | 108 +++++++++++++++++++++++++++++++++++++
>> fs/internal.h | 1 +
>> fs/namespace.c | 97 +++++++++++++++++++++++++--------
>> fs/super.c | 12 +++++
>> include/linux/fs_context.h | 1 +
>> include/linux/mount.h | 1 +
>> 6 files changed, 197 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/fs_context.c b/fs/fs_context.c
>> index 76f34f3d468ea..18ffe23ad4847 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,64 @@ 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 option string plus the MS_* superblock flag
>> names against
>> + * the mounted superblock's device. 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();
>> + 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;
>> +
>> + if (fc->ve_final_opts && *fc->ve_final_opts) {
>> + ssize_t ret = strscpy(page, fc->ve_final_opts, 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';
>> + err = ve_devmnt_verify(ve, fc->root->d_sb->s_dev, page, new_mount);
>> +
>> +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 +482,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);
>
> Some file systems override parse_monolithic, for example
> smb3_fs_context_parse_monolithic.
> They won't have ve_final_opts set, so checks may fail.
Agree with this, ve_devmnt_verify_fc in my opinion should bail in case
fc->ve_final_opts == NULL,
what do you think?
>
>> + fc->ve_final_opts = kstrdup(options, GFP_KERNEL);
>> + if (!fc->ve_final_opts) {
>> + if (options != options_orig)
>> + free_page((unsigned long)options);
>> + return -ENOMEM;
>> + }
>> + }
>> }
>> /*
>> @@ -742,6 +847,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 +1068,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 3647ce69b2c7a..e33d3ae70ccee 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 0f4a3668e558d..3a457588e96d9 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -3258,6 +3258,79 @@ 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)
>
> All these arguments should be const.
Not all, just cur and p.
>> +{
>> + 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 MS_* flag names to vet
>> + * @new_mount: true for a new mount, false for a remount
>> + *
>> + * 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)
>> +{
>> + struct ve_devmnt *devmnt;
>> + char *allowed = NULL, *hidden = NULL;
>> + int err;
>> +
>> + 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;
>> + break;
>> + }
>> + }
>
> What if the device is not in devmnt_list and user passed no options?
> opts is empty and allowed and hidden are NULLs, so all the following
> checks pass.
This is correct and consistent with what happens in ve_devmnt_process.
Look at commit
e7176b8050be ("fs: namespace: allow mounting blockdevices without extra
options")
For mount tries were device is not in device list (allowed and hidden
doesn't exist) the check passes
>
>> +
>> + /* 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
>> filesystem */
>> + 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));
>
> Is there a way to know that we lack of hidden mount options?
> With this approach, they are not hidden anymore, they are equal to
> they are required now.
>
> And with left ve_devmnt_process "user should provide these flags" is
> not working properly - in do_new_mount parse_monolithic_mount_data goes
> before vfs_get_tree, so ve_devmnt_process will add hidden_options
> instead of user.
>
> Maybe we should rename it to required_options or keep them as actually
> hidden?
Hidden options are actually hidden. They are added by the container
creator and are inserted
to the user-passed options in ve_devmnt_insert. This mean that if hidden
options is "nodev,nosuid"
and container tries to mount this block device with "rw,relatime" then
the new mount will be
"rw,relatime,nodev,nosuid" so the name is consistent with the purpose.
>> + return err;
>> +}
>> +
>> static inline int ve_mount_allowed(void)
>> {
>> struct ve_struct *ve = get_exec_env();
>> @@ -3308,23 +3381,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 +3413,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 +3866,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 1adebbf358032..c0c067eb2d8e1 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 1801aed1da67c..3dabb5ba11511 100644
>> --- a/include/linux/fs_context.h
>> +++ b/include/linux/fs_context.h
>> @@ -93,6 +93,7 @@ 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 */
>> + char *ve_final_opts; /* option string handed to
>> the fs, for the ve_devmnt policy check */
>> 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 0cbc6f6893c01..12898c96cc768 100644
>> --- a/include/linux/mount.h
>> +++ b/include/linux/mount.h
>> @@ -127,5 +127,6 @@ 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 *, dev_t, char *, bool);
>> #endif /* _LINUX_MOUNT_H */
>
> --
> Best regards, Riabchun Vladimir
> Linux Kernel Developer, Virtuozzo
>
--
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.
More information about the Devel
mailing list