On Fri, Jun 9, 2023 at 6:42 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> We recently ported util-linux to the new mount api. Now the mount(8)
> tool will by default use the new mount api. While trying hard to fall
> back to the old mount api gracefully there are still cases where we run
> into issues that are difficult to handle nicely.
>
> Now with mount(8) and libmount supporting the new mount api I expect an
> increase in the number of bug reports and issues we're going to see with
> filesystems that don't yet support the new mount api. So it's time we
> rectify this.
>
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> ---
> fs/overlayfs/super.c | 515 ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 279 insertions(+), 236 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index f97ad8b40dbb..ceaf05743f45 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -16,6 +16,8 @@
> #include <linux/posix_acl_xattr.h>
> #include <linux/exportfs.h>
> #include <linux/file.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
> #include "overlayfs.h"
>
> MODULE_AUTHOR("Miklos Szeredi <miklos@xxxxxxxxxx>");
> @@ -67,6 +69,59 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
> MODULE_PARM_DESC(metacopy,
> "Default to on or off for the metadata only copy up feature");
>
> +enum {
> + Opt_lowerdir,
> + Opt_upperdir,
> + Opt_workdir,
> + Opt_default_permissions,
> + Opt_redirect_dir,
> + Opt_index,
> + Opt_uuid,
> + Opt_nfs_export,
> + Opt_userxattr,
> + Opt_xino,
> + Opt_metacopy,
> + Opt_volatile,
> +};
> +
> +static const struct constant_table ovl_parameter_bool[] = {
> + { "on", true },
> + { "off", false },
> + {}
> +};
> +
> +static const struct constant_table ovl_parameter_xino[] = {
> + { "on", OVL_XINO_ON },
> + { "off", OVL_XINO_OFF },
> + { "auto", OVL_XINO_AUTO },
> + {}
> +};
> +
> +static const struct fs_parameter_spec ovl_parameter_spec[] = {
> + fsparam_string("lowerdir", Opt_lowerdir),
> + fsparam_string("upperdir", Opt_upperdir),
> + fsparam_string("workdir", Opt_workdir),
> + fsparam_flag("default_permissions", Opt_default_permissions),
> + fsparam_enum("redirect_dir", Opt_redirect_dir, ovl_parameter_bool),
> + fsparam_enum("index", Opt_index, ovl_parameter_bool),
> + fsparam_enum("uuid", Opt_uuid, ovl_parameter_bool),
> + fsparam_enum("nfs_export", Opt_nfs_export, ovl_parameter_bool),
> + fsparam_flag("userxattr", Opt_userxattr),
> + fsparam_enum("xino", Opt_xino, ovl_parameter_xino),
> + fsparam_enum("metacopy", Opt_metacopy, ovl_parameter_bool),
> + fsparam_flag("volatile", Opt_volatile),
> + {}
> +};
> +
> +#define OVL_METACOPY_SET BIT(0)
> +#define OVL_REDIRECT_SET BIT(1)
> +#define OVL_NFS_EXPORT_SET BIT(2)
> +#define OVL_INDEX_SET BIT(3)
> +
> +struct ovl_fs_context {
> + u8 set;
> +};
> +
> static void ovl_dentry_release(struct dentry *dentry)
> {
> struct ovl_entry *oe = dentry->d_fsdata;
> @@ -394,27 +449,6 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
> return 0;
> }
>
> -static int ovl_remount(struct super_block *sb, int *flags, char *data)
> -{
> - struct ovl_fs *ofs = sb->s_fs_info;
> - struct super_block *upper_sb;
> - int ret = 0;
> -
> - if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
> - return -EROFS;
> -
> - if (*flags & SB_RDONLY && !sb_rdonly(sb)) {
> - upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
> - if (ovl_should_sync(ofs)) {
> - down_read(&upper_sb->s_umount);
> - ret = sync_filesystem(upper_sb);
> - up_read(&upper_sb->s_umount);
> - }
> - }
> -
> - return ret;
> -}
> -
> static const struct super_operations ovl_super_operations = {
> .alloc_inode = ovl_alloc_inode,
> .free_inode = ovl_free_inode,
> @@ -424,76 +458,8 @@ static const struct super_operations ovl_super_operations = {
> .sync_fs = ovl_sync_fs,
> .statfs = ovl_statfs,
> .show_options = ovl_show_options,
> - .remount_fs = ovl_remount,
> -};
> -
> -enum {
> - OPT_LOWERDIR,
> - OPT_UPPERDIR,
> - OPT_WORKDIR,
> - OPT_DEFAULT_PERMISSIONS,
> - OPT_REDIRECT_DIR,
> - OPT_INDEX_ON,
> - OPT_INDEX_OFF,
> - OPT_UUID_ON,
> - OPT_UUID_OFF,
> - OPT_NFS_EXPORT_ON,
> - OPT_USERXATTR,
> - OPT_NFS_EXPORT_OFF,
> - OPT_XINO_ON,
> - OPT_XINO_OFF,
> - OPT_XINO_AUTO,
> - OPT_METACOPY_ON,
> - OPT_METACOPY_OFF,
> - OPT_VOLATILE,
> - OPT_ERR,
> -};
> -
> -static const match_table_t ovl_tokens = {
> - {OPT_LOWERDIR, "lowerdir=%s"},
> - {OPT_UPPERDIR, "upperdir=%s"},
> - {OPT_WORKDIR, "workdir=%s"},
> - {OPT_DEFAULT_PERMISSIONS, "default_permissions"},
> - {OPT_REDIRECT_DIR, "redirect_dir=%s"},
> - {OPT_INDEX_ON, "index=on"},
> - {OPT_INDEX_OFF, "index=off"},
> - {OPT_USERXATTR, "userxattr"},
> - {OPT_UUID_ON, "uuid=on"},
> - {OPT_UUID_OFF, "uuid=off"},
> - {OPT_NFS_EXPORT_ON, "nfs_export=on"},
> - {OPT_NFS_EXPORT_OFF, "nfs_export=off"},
> - {OPT_XINO_ON, "xino=on"},
> - {OPT_XINO_OFF, "xino=off"},
> - {OPT_XINO_AUTO, "xino=auto"},
> - {OPT_METACOPY_ON, "metacopy=on"},
> - {OPT_METACOPY_OFF, "metacopy=off"},
> - {OPT_VOLATILE, "volatile"},
> - {OPT_ERR, NULL}
> };
>
> -static char *ovl_next_opt(char **s)
> -{
> - char *sbegin = *s;
> - char *p;
> -
> - if (sbegin == NULL)
> - return NULL;
> -
> - for (p = sbegin; *p; p++) {
> - if (*p == '\\') {
> - p++;
> - if (!*p)
> - break;
> - } else if (*p == ',') {
> - *p = '\0';
> - *s = p + 1;
> - return sbegin;
> - }
> - }
> - *s = NULL;
> - return sbegin;
> -}
> -
> static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
> {
> if (strcmp(mode, "on") == 0) {
> @@ -517,123 +483,14 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
> return 0;
> }
>
> -static int ovl_parse_opt(char *opt, struct ovl_config *config)
> +static int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> + struct ovl_config *config)
> {
> - char *p;
> int err;
> - bool metacopy_opt = false, redirect_opt = false;
> - bool nfs_export_opt = false, index_opt = false;
> -
> - config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
> - if (!config->redirect_mode)
> - return -ENOMEM;
> -
> - while ((p = ovl_next_opt(&opt)) != NULL) {
> - int token;
> - substring_t args[MAX_OPT_ARGS];
> -
> - if (!*p)
> - continue;
> -
> - token = match_token(p, ovl_tokens, args);
> - switch (token) {
> - case OPT_UPPERDIR:
> - kfree(config->upperdir);
> - config->upperdir = match_strdup(&args[0]);
> - if (!config->upperdir)
> - return -ENOMEM;
> - break;
> -
> - case OPT_LOWERDIR:
> - kfree(config->lowerdir);
> - config->lowerdir = match_strdup(&args[0]);
> - if (!config->lowerdir)
> - return -ENOMEM;
> - break;
> -
> - case OPT_WORKDIR:
> - kfree(config->workdir);
> - config->workdir = match_strdup(&args[0]);
> - if (!config->workdir)
> - return -ENOMEM;
> - break;
> -
> - case OPT_DEFAULT_PERMISSIONS:
> - config->default_permissions = true;
> - break;
> -
> - case OPT_REDIRECT_DIR:
> - kfree(config->redirect_mode);
> - config->redirect_mode = match_strdup(&args[0]);
> - if (!config->redirect_mode)
> - return -ENOMEM;
> - redirect_opt = true;
> - break;
> -
> - case OPT_INDEX_ON:
> - config->index = true;
> - index_opt = true;
> - break;
> -
> - case OPT_INDEX_OFF:
> - config->index = false;
> - index_opt = true;
> - break;
> -
> - case OPT_UUID_ON:
> - config->uuid = true;
> - break;
> -
> - case OPT_UUID_OFF:
> - config->uuid = false;
> - break;
> -
> - case OPT_NFS_EXPORT_ON:
> - config->nfs_export = true;
> - nfs_export_opt = true;
> - break;
> -
> - case OPT_NFS_EXPORT_OFF:
> - config->nfs_export = false;
> - nfs_export_opt = true;
> - break;
> -
> - case OPT_XINO_ON:
> - config->xino = OVL_XINO_ON;
> - break;
> -
> - case OPT_XINO_OFF:
> - config->xino = OVL_XINO_OFF;
> - break;
> -
> - case OPT_XINO_AUTO:
> - config->xino = OVL_XINO_AUTO;
> - break;
> -
> - case OPT_METACOPY_ON:
> - config->metacopy = true;
> - metacopy_opt = true;
> - break;
> -
> - case OPT_METACOPY_OFF:
> - config->metacopy = false;
> - metacopy_opt = true;
> - break;
> -
> - case OPT_VOLATILE:
> - config->ovl_volatile = true;
> - break;
> -
> - case OPT_USERXATTR:
> - config->userxattr = true;
> - break;
> -
> - default:
> - pr_err("unrecognized mount option \"%s\" or missing value\n",
> - p);
> - return -EINVAL;
> - }
> - }
> + bool metacopy_opt = ctx->set & OVL_METACOPY_SET,
> + redirect_opt = ctx->set & OVL_REDIRECT_SET;
> + bool nfs_export_opt = ctx->set & OVL_NFS_EXPORT_SET,
> + index_opt = ctx->set & OVL_INDEX_SET;
Nit: please split lines here.
>
> /* Workdir/index are useless in non-upper mount */
> if (!config->upperdir) {
> @@ -1882,12 +1739,143 @@ static struct dentry *ovl_get_root(struct super_block *sb,
> return root;
> }
>
> -static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> +static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
> {
> - struct path upperpath = { };
> + int err = 0;
> + struct fs_parse_result result;
> + struct ovl_fs *ofs = fc->s_fs_info;
> + struct ovl_config *config = &ofs->config;
> + struct ovl_fs_context *ctx = fc->fs_private;
> + struct path path;
> + char *dup;
> + int opt;
> + char *sval;
> +
> + /*
> + * On remount overlayfs has always ignored all mount options no
> + * matter if malformed or not so for backwards compatibility we
> + * do the same here.
> + */
> + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> + return 0;
> +
> + opt = fs_parse(fc, ovl_parameter_spec, param, &result);
> + if (opt < 0)
> + return opt;
> +
> + switch (opt) {
> + case Opt_lowerdir:
> + dup = kstrdup(param->string, GFP_KERNEL);
> + if (!dup) {
> + path_put(&path);
> + err = -ENOMEM;
> + break;
> + }
> +
> + kfree(config->lowerdir);
> + config->lowerdir = dup;
> + break;
> + case Opt_upperdir:
> + dup = kstrdup(param->string, GFP_KERNEL);
> + if (!dup) {
> + path_put(&path);
> + err = -ENOMEM;
> + break;
> + }
> +
> + kfree(config->upperdir);
> + config->upperdir = dup;
> + break;
> + case Opt_workdir:
> + dup = kstrdup(param->string, GFP_KERNEL);
> + if (!dup) {
> + path_put(&path);
> + err = -ENOMEM;
> + break;
> + }
> +
> + kfree(config->workdir);
> + config->workdir = dup;
> + break;
> + case Opt_default_permissions:
> + config->default_permissions = true;
> + break;
> + case Opt_index:
> + config->index = result.uint_32;
> + ctx->set |= OVL_INDEX_SET;
> + break;
> + case Opt_uuid:
> + config->uuid = result.uint_32;
> + break;
> + case Opt_nfs_export:
> + config->nfs_export = result.uint_32;
> + ctx->set |= OVL_NFS_EXPORT_SET;
> + break;
> + case Opt_metacopy:
> + config->metacopy = result.uint_32;
> + ctx->set |= OVL_METACOPY_SET;
> + break;
> + case Opt_userxattr:
> + config->userxattr = true;
> + break;
> + case Opt_volatile:
> + config->ovl_volatile = true;
> + break;
> + case Opt_xino:
> + config->xino = result.uint_32;
> + break;
> + case Opt_redirect_dir:
> + if (result.uint_32 == true)
> + sval = kstrdup("on", GFP_KERNEL);
> + else
> + sval = kstrdup("off", GFP_KERNEL);
> + if (!sval) {
> + err = -ENOMEM;
> + break;
> + }
> +
> + kfree(config->redirect_mode);
> + config->redirect_mode = sval;
> + ctx->set |= OVL_REDIRECT_SET;
> + break;
> + default:
> + pr_err("unrecognized mount option \"%s\" or missing value\n", param->key);
> + return -EINVAL;
> + }
> +
> + return err;
> +}
For the end result, all these functions above should be in params.c
I don't mind if you move it with an extra patch at the beginning
Probably easier at the end. Just as long as you do not move
ovl_fs_params_verify() in the same patch that changes half of it.
Thanks,
Amir.
