On 6/2/26 10:14, Vasileios Almpanis wrote:
> In legacy mount callpaths, userspace might pass mount options as
> flags. These flags escape our checks in ve_devmnt_process allowing
> devices to be mounted inside containers with options not specified in
> the allowed field. Introduce helpers that take these flags and
> already existing tables of flag -> string representation to construct
> a comma separated value string from them, and append them to userspace
> provided data. Then pass this string to parse_monolithic_mount_data
> enforcing the same checks symmetrically in both mount and fsconfig
> syscalls.
>
> In the remount path, run legacy_merge_mount_data() before
> ve_devmnt_process() so container device mount policy sees MS_* flags
> from the legacy mount(2) API, not only the user-supplied option string.
> Keep ve_prepare_mount_options() for legacy parsers that do not use
> generic_parse_monolithic().
>
> https://virtuozzo.atlassian.net/browse/VSTOR-132330
> Signed-off-by: Vasileios Almpanis <[email protected]>
>
> Feature: ve: ve generic structures
> ---
> fs/fs_context.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/internal.h | 8 ++++
> fs/namespace.c | 33 +++++++++++----
> 3 files changed, 139 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fs_context.c b/fs/fs_context.c
> index 76f34f3d468e..65495421fa9e 100644
> --- a/fs/fs_context.c
> +++ b/fs/fs_context.c
> @@ -81,6 +81,111 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const
> char *key)
> return -ENOPARAM;
> }
>
> +#ifdef CONFIG_VE
> +/*
> + * Format fc->sb_flags as comma-separated mount option names (ro, sync, ...).
> + * Matches vfs_parse_sb_flag() / common_{set,clear}_sb_flag tables.
> + */
> +static int vfs_format_sb_flags(struct fs_context *fc, char *buf, size_t
> buflen)
> +{
> + const struct constant_table *p;
> + unsigned int flags = fc->sb_flags;
> + unsigned int mask = fc->sb_flags_mask;
> + size_t len = 0;
> + int token;
> +
> + for (p = common_set_sb_flag; p->name; p++) {
> + size_t namelen = strlen(p->name);
> +
> + token = p->value;
> + if (!(flags & token))
> + continue;
> + if (len + namelen + 2 > buflen)
> + return -ENOSPC;
> + if (len)
> + buf[len++] = ',';
> + memcpy(buf + len, p->name, namelen);
> + len += namelen;
> + }
> +
> + for (p = common_clear_sb_flag; p->name; p++) {
> + size_t namelen = strlen(p->name);
> +
> + token = p->value;
> + if (!(mask & token) || (flags & token))
> + continue;
> + if (len + namelen + 2 > buflen)
> + return -ENOSPC;
> + if (len)
> + buf[len++] = ',';
> + memcpy(buf + len, p->name, namelen);
> + len += namelen;
> + }
> +
> + buf[len] = '\0';
> + return len;
> +}
> +
> +/*
> + * For legacy mount(2), MS_* mount flags are folded into fc->sb_flags and are
> + * not present in the monolithic data string. Build a page holding those
> + * options plus any user data for ve_devmnt checks in
> vfs_parse_monolithic_sep.
> + *
> + * Returns @data when nothing needs to be added, a new page otherwise, or
> + * ERR_PTR() on failure. The caller must free_page() when the result !=
> @data.
> + */
> +void *legacy_merge_mount_data(struct fs_context *fc, void *data)
> +{
> + struct ve_struct *ve = get_exec_env();
> + char flags_buf[128];
> + size_t data_len = 0;
> + size_t total;
> + char *page, *p;
> + int flags_len;
> +
> + if (ve_is_super(ve))
> + return data;
> +
> + if (!fc->fs_type || !(fc->fs_type->fs_flags & FS_REQUIRES_DEV))
> + return data;
> +
> + if (fc->fs_type->fs_flags & FS_BINARY_MOUNTDATA)
Maybe worth a comment that some filesystems (like btrfs) use binary
mount data and thus can probably side-pass our checks.
> + return data;
> +
> + flags_len = vfs_format_sb_flags(fc, flags_buf, sizeof(flags_buf));
> + if (flags_len < 0)
> + return ERR_PTR(flags_len);
> +
> + if (!flags_len)
> + return data;
> +
> + if (data && *(char *)data)
> + data_len = strlen(data);
> +
> + total = flags_len + 1;
> + if (data_len)
> + total += data_len + 1;
> +
> + if (total > PAGE_SIZE)
> + return ERR_PTR(-EINVAL);
> +
> + page = (char *)__get_free_page(GFP_KERNEL);
> + if (!page)
> + return ERR_PTR(-ENOMEM);
> +
> + p = page;
> + memcpy(p, flags_buf, flags_len);
> + p += flags_len;
> + if (data_len) {
> + *p++ = ',';
> + memcpy(p, data, data_len);
> + p += data_len;
> + }
> + *p = '\0';
maybe a new line here
> + return page;
> +}
> +#endif /* CONFIG_VE */
> +
> /**
> * vfs_parse_fs_param_source - Handle setting "source" via parameter
> * @fc: The filesystem context to modify
> diff --git a/fs/internal.h b/fs/internal.h
> index 0064345cb9d0..120c239df394 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -46,6 +46,14 @@ 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 *);
> +#ifdef CONFIG_VE
> +void *legacy_merge_mount_data(struct fs_context *fc, void *data);
> +#else
> +static inline void *legacy_merge_mount_data(struct fs_context *fc, void
> *data)
> +{
> + return data;
> +}
> +#endif
> 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 acd4507e1247..27a72d4cb245 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3295,6 +3295,7 @@ static int do_remount(struct path *path, int ms_flags,
> int sb_flags,
> struct super_block *sb = path->mnt->mnt_sb;
> struct mount *mnt = real_mount(path->mnt);
> struct fs_context *fc;
> + void *mnt_data = NULL;
>
> if (!check_mnt(mnt))
> return -EINVAL;
> @@ -3315,13 +3316,18 @@ 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;
> + mnt_data = legacy_merge_mount_data(fc, data);
> + if (IS_ERR(mnt_data)) {
> + err = PTR_ERR(mnt_data);
> + goto err;
> }
>
> - err = parse_monolithic_mount_data(fc, data);
> + err = ve_prepare_mount_options(fc, mnt_data);
> + if (err)
> + goto free_mnt_data;
> +
> + err = parse_monolithic_mount_data(fc, mnt_data);
> +
nit: excess newline
> if (!err) {
> down_write(&sb->s_umount);
> err = -EPERM;
> @@ -3338,6 +3344,10 @@ static int do_remount(struct path *path, int ms_flags,
> int sb_flags,
>
> mnt_warn_timestamp_expiry(path, &mnt->mnt);
>
> +free_mnt_data:
> + if (mnt_data && mnt_data != data)
> + free_page((unsigned long)mnt_data);
> +err:
> put_fs_context(fc);
> return err;
> }
> @@ -3774,8 +3784,17 @@ 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));
> - if (!err)
> - err = parse_monolithic_mount_data(fc, data);
> + if (!err) {
> + void *mnt_data = legacy_merge_mount_data(fc, data);
> +
> + if (IS_ERR(mnt_data)) {
> + err = PTR_ERR(mnt_data);
> + } else {
> + err = parse_monolithic_mount_data(fc, mnt_data);
> + if (mnt_data != data)
> + free_page((unsigned long)mnt_data);
> + }
> + }
Why do we change this codepath? We don't have any vz-specific code (e.g.
ve_prepare_mount_options()) here.
> if (!err && !mount_capable(fc))
> err = -EPERM;
> if (!err)
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
