| Message ID | 20250214-work-overlayfs-v1-1-465d1867d3d4@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ovl: add override_creds mount option | expand |
On Fri, Feb 14, 2025 at 5:46 PM Christian Brauner <brauner@kernel.org> wrote: > > Currently overlayfs uses the mounter's credentials for it's > override_creds() calls. That provides a consistent permission model. > > This patches allows a caller to instruct overlayfs to use its > credentials instead. The caller must be located in the same user > namespace as the user namespace the overlayfs instance will be mounted > in. This provides a consistent and simple security model. > > With this it is possible to e.g., mount an overlayfs instance where the > mounter must have CAP_SYS_ADMIN but the credentials used for > override_creds() have dropped CAP_SYS_ADMIN. It also allows the usage of > custom fs{g,u}id different from the callers and other tweaks. > > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > fs/overlayfs/ovl_entry.h | 1 + > fs/overlayfs/params.c | 25 +++++++++++++++++++++++++ > fs/overlayfs/super.c | 13 ++++++++++++- > 3 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h > index cb449ab310a7..ed45553943e1 100644 > --- a/fs/overlayfs/ovl_entry.h > +++ b/fs/overlayfs/ovl_entry.h > @@ -19,6 +19,7 @@ struct ovl_config { > bool metacopy; > bool userxattr; > bool ovl_volatile; > + bool ovl_credentials; IMO this is not a configuration... > }; > > struct ovl_sb { > diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c > index 1115c22deca0..5dad23d6f121 100644 > --- a/fs/overlayfs/params.c > +++ b/fs/overlayfs/params.c > @@ -59,6 +59,7 @@ enum ovl_opt { > Opt_metacopy, > Opt_verity, > Opt_volatile, > + Opt_override_creds, > }; > > static const struct constant_table ovl_parameter_bool[] = { > @@ -155,6 +156,7 @@ const struct fs_parameter_spec ovl_parameter_spec[] = { > fsparam_enum("metacopy", Opt_metacopy, ovl_parameter_bool), > fsparam_enum("verity", Opt_verity, ovl_parameter_verity), > fsparam_flag("volatile", Opt_volatile), > + fsparam_flag_no("override_creds", Opt_override_creds), > {} > }; > > @@ -662,6 +664,27 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param) > case Opt_userxattr: > config->userxattr = true; > break; > + case Opt_override_creds: { > + const struct cred *cred = ofs->creator_cred; > + > + if (!result.negated) { > + if (fc->user_ns != current_user_ns()) { > + err = -EINVAL; > + break; > + } > + > + ofs->creator_cred = prepare_creds(); > + if (!ofs->creator_cred) > + err = -EINVAL; > + else > + config->ovl_credentials = true; > + } else { > + ofs->creator_cred = NULL; > + config->ovl_credentials = false; > + } > + put_cred(cred); > + break; > + } > default: > pr_err("unrecognized mount option \"%s\" or missing value\n", > param->key); > @@ -1071,5 +1094,7 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry) > if (ofs->config.verity_mode != ovl_verity_mode_def()) > seq_printf(m, ",verity=%s", > ovl_verity_mode(&ofs->config)); > + if (ofs->config.ovl_credentials) > + seq_puts(m, ",override_creds"); ...and there is no meaning to showing it in mount options, because this is not a replayable configuration. If you strap ",override_creds" to a normal shell command line mount it means nothing and showing this option does not carry any information about how (in which context) it was executed. I am willing to be convinced otherwise. > return 0; > } > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 86ae6f6da36b..157ab9e8f6f8 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -654,6 +654,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, > const struct path *workpath) > { > struct vfsmount *mnt = ovl_upper_mnt(ofs); > + const struct cred *old_cred; > struct dentry *workdir; > struct file *tmpfile; > bool rename_whiteout; > @@ -665,6 +666,8 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, > if (err) > return err; > > + old_cred = ovl_override_creds(sb); > + > workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false); > err = PTR_ERR(workdir); > if (IS_ERR_OR_NULL(workdir)) > @@ -788,6 +791,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, > ofs->config.nfs_export = false; > } > out: > + ovl_revert_creds(old_cred); > mnt_drop_write(mnt); > return err; > } > @@ -830,6 +834,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, > struct ovl_entry *oe, const struct path *upperpath) > { > struct vfsmount *mnt = ovl_upper_mnt(ofs); > + const struct cred *old_cred; > struct dentry *indexdir; > struct dentry *origin = ovl_lowerstack(oe)->dentry; > const struct ovl_fh *fh; > @@ -843,6 +848,8 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, > if (err) > goto out_free_fh; > > + old_cred = ovl_override_creds(sb); > + > /* Verify lower root is upper root origin */ > err = ovl_verify_origin_fh(ofs, upperpath->dentry, fh, true); > if (err) { > @@ -893,6 +900,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, > pr_warn("try deleting index dir or mounting with '-o index=off' to disable inodes index.\n"); > > out: > + ovl_revert_creds(old_cred); > mnt_drop_write(mnt); > out_free_fh: > kfree(fh); > @@ -1318,7 +1326,10 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) > sb->s_d_op = &ovl_dentry_operations; > > err = -ENOMEM; > - ofs->creator_cred = cred = prepare_creds(); > + if (!ofs->creator_cred) > + ofs->creator_cred = cred = prepare_creds(); > + else > + cred = (struct cred *)ofs->creator_cred; > if (!cred) > goto out_err; > Is there any reason not to scope the rest of ovl_fill_super() with the alternative override_creds instead of scoping helper functions? If there is a reason then I do not see what it is. My opinion is that it does not change the permission model if we switch one mounter_creds with another, as long as we execute all operations on underlying layers with a consistent set of creds (give or take CAP_SYS_RESOURCE). If we scope the entire ovl_fill_super() with the override_creds used later on, then we get this consistent property. I should note that before the new mount API, it was obvious that all operations on underlying layers are performed with "mounter creds". with new API, "mounter creds" are really "fsconfig setup creds", and kern_path() doing lookup when parsing string *dir= mount options may be executed with different creds than fsconfig setup creds. So the way I see it, the override_creds option is more of a command, which allows better control over the single consistent set of ovl creds. Thanks, Amir.
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h index cb449ab310a7..ed45553943e1 100644 --- a/fs/overlayfs/ovl_entry.h +++ b/fs/overlayfs/ovl_entry.h @@ -19,6 +19,7 @@ struct ovl_config { bool metacopy; bool userxattr; bool ovl_volatile; + bool ovl_credentials; }; struct ovl_sb { diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c index 1115c22deca0..5dad23d6f121 100644 --- a/fs/overlayfs/params.c +++ b/fs/overlayfs/params.c @@ -59,6 +59,7 @@ enum ovl_opt { Opt_metacopy, Opt_verity, Opt_volatile, + Opt_override_creds, }; static const struct constant_table ovl_parameter_bool[] = { @@ -155,6 +156,7 @@ const struct fs_parameter_spec ovl_parameter_spec[] = { fsparam_enum("metacopy", Opt_metacopy, ovl_parameter_bool), fsparam_enum("verity", Opt_verity, ovl_parameter_verity), fsparam_flag("volatile", Opt_volatile), + fsparam_flag_no("override_creds", Opt_override_creds), {} }; @@ -662,6 +664,27 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param) case Opt_userxattr: config->userxattr = true; break; + case Opt_override_creds: { + const struct cred *cred = ofs->creator_cred; + + if (!result.negated) { + if (fc->user_ns != current_user_ns()) { + err = -EINVAL; + break; + } + + ofs->creator_cred = prepare_creds(); + if (!ofs->creator_cred) + err = -EINVAL; + else + config->ovl_credentials = true; + } else { + ofs->creator_cred = NULL; + config->ovl_credentials = false; + } + put_cred(cred); + break; + } default: pr_err("unrecognized mount option \"%s\" or missing value\n", param->key); @@ -1071,5 +1094,7 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry) if (ofs->config.verity_mode != ovl_verity_mode_def()) seq_printf(m, ",verity=%s", ovl_verity_mode(&ofs->config)); + if (ofs->config.ovl_credentials) + seq_puts(m, ",override_creds"); return 0; } diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 86ae6f6da36b..157ab9e8f6f8 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -654,6 +654,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, const struct path *workpath) { struct vfsmount *mnt = ovl_upper_mnt(ofs); + const struct cred *old_cred; struct dentry *workdir; struct file *tmpfile; bool rename_whiteout; @@ -665,6 +666,8 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, if (err) return err; + old_cred = ovl_override_creds(sb); + workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false); err = PTR_ERR(workdir); if (IS_ERR_OR_NULL(workdir)) @@ -788,6 +791,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs, ofs->config.nfs_export = false; } out: + ovl_revert_creds(old_cred); mnt_drop_write(mnt); return err; } @@ -830,6 +834,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, struct ovl_entry *oe, const struct path *upperpath) { struct vfsmount *mnt = ovl_upper_mnt(ofs); + const struct cred *old_cred; struct dentry *indexdir; struct dentry *origin = ovl_lowerstack(oe)->dentry; const struct ovl_fh *fh; @@ -843,6 +848,8 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, if (err) goto out_free_fh; + old_cred = ovl_override_creds(sb); + /* Verify lower root is upper root origin */ err = ovl_verify_origin_fh(ofs, upperpath->dentry, fh, true); if (err) { @@ -893,6 +900,7 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, pr_warn("try deleting index dir or mounting with '-o index=off' to disable inodes index.\n"); out: + ovl_revert_creds(old_cred); mnt_drop_write(mnt); out_free_fh: kfree(fh); @@ -1318,7 +1326,10 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_d_op = &ovl_dentry_operations; err = -ENOMEM; - ofs->creator_cred = cred = prepare_creds(); + if (!ofs->creator_cred) + ofs->creator_cred = cred = prepare_creds(); + else + cred = (struct cred *)ofs->creator_cred; if (!cred) goto out_err;
Currently overlayfs uses the mounter's credentials for it's override_creds() calls. That provides a consistent permission model. This patches allows a caller to instruct overlayfs to use its credentials instead. The caller must be located in the same user namespace as the user namespace the overlayfs instance will be mounted in. This provides a consistent and simple security model. With this it is possible to e.g., mount an overlayfs instance where the mounter must have CAP_SYS_ADMIN but the credentials used for override_creds() have dropped CAP_SYS_ADMIN. It also allows the usage of custom fs{g,u}id different from the callers and other tweaks. Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/overlayfs/ovl_entry.h | 1 + fs/overlayfs/params.c | 25 +++++++++++++++++++++++++ fs/overlayfs/super.c | 13 ++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-)