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 <[email protected]>

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))
-- 
2.43.0

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to