From f37b334728814b14745e15d46f9eab73750b67ec Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:38 +0100 Subject: ovl: add ovl_override_creator_creds cred guard The current code to override credentials for creation operations is pretty difficult to understand. We effectively override the credentials twice: (1) override with the mounter's credentials (2) copy the mounts credentials and override the fs{g,u}id with the inode {u,g}id And then we elide the revert because it would be an idempotent revert. That elision doesn't buy us anything anymore though because I've made it all work without any reference counting anyway. All it does is mix the two credential overrides together. We can use a cleanup guard to clarify the creation codepaths and make them easier to understand. This just introduces the cleanup guard keeping the patch reviewable. We'll convert the caller in follow-up patches and then drop the duplicated code. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-1-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 8e8ede6a1217..c61976180a83 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -581,6 +581,42 @@ out_cleanup_unlocked: goto out_dput; } +static const struct cred *ovl_override_creator_creds(struct dentry *dentry, struct inode *inode, umode_t mode) +{ + int err; + + if (WARN_ON_ONCE(current->cred != ovl_creds(dentry->d_sb))) + return ERR_PTR(-EINVAL); + + CLASS(prepare_creds, override_cred)(); + if (!override_cred) + return ERR_PTR(-ENOMEM); + + override_cred->fsuid = inode->i_uid; + override_cred->fsgid = inode->i_gid; + + err = security_dentry_create_files_as(dentry, mode, &dentry->d_name, + current->cred, override_cred); + if (err) + return ERR_PTR(err); + + return override_creds(no_free_ptr(override_cred)); +} + +static void ovl_revert_creator_creds(const struct cred *old_cred) +{ + const struct cred *override_cred; + + override_cred = revert_creds(old_cred); + put_cred(override_cred); +} + +DEFINE_CLASS(ovl_override_creator_creds, + const struct cred *, + if (!IS_ERR_OR_NULL(_T)) ovl_revert_creator_creds(_T), + ovl_override_creator_creds(dentry, inode, mode), + struct dentry *dentry, struct inode *inode, umode_t mode) + static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode, umode_t mode, -- cgit v1.2.3 From 8d7fc461e45abc7f67c455d908a2e709dec9e3b9 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:39 +0100 Subject: ovl: port ovl_create_tmpfile() to new ovl_override_creator_creds cleanup guard This clearly indicates the double-credential override and makes the code a lot easier to grasp with one glance. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-2-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index c61976180a83..68f6617b6a77 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -1381,7 +1381,6 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, struct inode *inode, umode_t mode) { - const struct cred *new_cred __free(put_cred) = NULL; struct path realparentpath; struct file *realfile; struct ovl_file *of; @@ -1390,33 +1389,34 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, int flags = file->f_flags | OVL_OPEN_FLAGS; int err; - scoped_class(override_creds_ovl, old_cred, dentry->d_sb) { - new_cred = ovl_setup_cred_for_create(dentry, inode, mode, old_cred); - if (IS_ERR(new_cred)) - return PTR_ERR(new_cred); - - ovl_path_upper(dentry->d_parent, &realparentpath); - realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath, - mode, current_cred()); - err = PTR_ERR_OR_ZERO(realfile); - pr_debug("tmpfile/open(%pd2, 0%o) = %i\n", realparentpath.dentry, mode, err); - if (err) - return err; + with_ovl_creds(dentry->d_sb) { + scoped_class(ovl_override_creator_creds, cred, dentry, inode, mode) { + if (IS_ERR(cred)) + return PTR_ERR(cred); + + ovl_path_upper(dentry->d_parent, &realparentpath); + realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath, + mode, current_cred()); + err = PTR_ERR_OR_ZERO(realfile); + pr_debug("tmpfile/open(%pd2, 0%o) = %i\n", realparentpath.dentry, mode, err); + if (err) + return err; - of = ovl_file_alloc(realfile); - if (!of) { - fput(realfile); - return -ENOMEM; - } + of = ovl_file_alloc(realfile); + if (!of) { + fput(realfile); + return -ENOMEM; + } - /* ovl_instantiate() consumes the newdentry reference on success */ - newdentry = dget(realfile->f_path.dentry); - err = ovl_instantiate(dentry, inode, newdentry, false, file); - if (!err) { - file->private_data = of; - } else { - dput(newdentry); - ovl_file_free(of); + /* ovl_instantiate() consumes the newdentry reference on success */ + newdentry = dget(realfile->f_path.dentry); + err = ovl_instantiate(dentry, inode, newdentry, false, file); + if (!err) { + file->private_data = of; + } else { + dput(newdentry); + ovl_file_free(of); + } } } return err; -- cgit v1.2.3 From d6ef072d09b2341e606aeeaf14c3510dec329c63 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:40 +0100 Subject: ovl: reflow ovl_create_or_link() Reflow the creation routine in preparation of porting it to a guard. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-3-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 51 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 68f6617b6a77..9eafddea8192 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -650,6 +650,16 @@ static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry, return override_cred; } +static int ovl_create_handle_whiteouts(struct dentry *dentry, + struct inode *inode, + struct ovl_cattr *attr) +{ + if (!ovl_dentry_is_whiteout(dentry)) + return ovl_create_upper(dentry, inode, attr); + + return ovl_create_over_whiteout(dentry, inode, attr); +} + static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, struct ovl_cattr *attr, bool origin) { @@ -668,29 +678,28 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, return err; } - if (!attr->hardlink) { - /* - * In the creation cases(create, mkdir, mknod, symlink), - * ovl should transfer current's fs{u,g}id to underlying - * fs. Because underlying fs want to initialize its new - * inode owner using current's fs{u,g}id. And in this - * case, the @inode is a new inode that is initialized - * in inode_init_owner() to current's fs{u,g}id. So use - * the inode's i_{u,g}id to override the cred's fs{u,g}id. - * - * But in the other hardlink case, ovl_link() does not - * create a new inode, so just use the ovl mounter's - * fs{u,g}id. - */ - new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred); - if (IS_ERR(new_cred)) - return PTR_ERR(new_cred); - } + /* + * In the creation cases(create, mkdir, mknod, symlink), + * ovl should transfer current's fs{u,g}id to underlying + * fs. Because underlying fs want to initialize its new + * inode owner using current's fs{u,g}id. And in this + * case, the @inode is a new inode that is initialized + * in inode_init_owner() to current's fs{u,g}id. So use + * the inode's i_{u,g}id to override the cred's fs{u,g}id. + * + * But in the other hardlink case, ovl_link() does not + * create a new inode, so just use the ovl mounter's + * fs{u,g}id. + */ + + if (attr->hardlink) + return ovl_create_handle_whiteouts(dentry, inode, attr); - if (!ovl_dentry_is_whiteout(dentry)) - return ovl_create_upper(dentry, inode, attr); + new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred); + if (IS_ERR(new_cred)) + return PTR_ERR(new_cred); - return ovl_create_over_whiteout(dentry, inode, attr); + return ovl_create_handle_whiteouts(dentry, inode, attr); } return err; } -- cgit v1.2.3 From 8a227c2766177db9733f86388d9d811df81e44ac Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:41 +0100 Subject: ovl: mark ovl_setup_cred_for_create() as unused temporarily The function will become unused in the next patch. We'll remove it in later patches to keep the diff legible. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-4-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 9eafddea8192..b21db1eb34bc 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -617,7 +617,7 @@ DEFINE_CLASS(ovl_override_creator_creds, ovl_override_creator_creds(dentry, inode, mode), struct dentry *dentry, struct inode *inode, umode_t mode) -static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry, +static const __maybe_unused struct cred *ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode, umode_t mode, const struct cred *old_cred) -- cgit v1.2.3 From e566bff963220ba0f740da42d46dd55c34ef745e Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:42 +0100 Subject: ovl: port ovl_create_or_link() to new ovl_override_creator_creds cleanup guard This clearly indicates the double-credential override and makes the code a lot easier to grasp with one glance. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-5-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index b21db1eb34bc..6cfa7857b352 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -664,10 +664,9 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, struct ovl_cattr *attr, bool origin) { int err; - const struct cred *new_cred __free(put_cred) = NULL; struct dentry *parent = dentry->d_parent; - scoped_class(override_creds_ovl, old_cred, dentry->d_sb) { + with_ovl_creds(dentry->d_sb) { /* * When linking a file with copy up origin into a new parent, mark the * new parent dir "impure". @@ -695,11 +694,11 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, if (attr->hardlink) return ovl_create_handle_whiteouts(dentry, inode, attr); - new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred); - if (IS_ERR(new_cred)) - return PTR_ERR(new_cred); - - return ovl_create_handle_whiteouts(dentry, inode, attr); + scoped_class(ovl_override_creator_creds, cred, dentry, inode, attr->mode) { + if (IS_ERR(cred)) + return PTR_ERR(cred); + return ovl_create_handle_whiteouts(dentry, inode, attr); + } } return err; } -- cgit v1.2.3 From 89a11f004f5e3806966cb0e522c4b975bbccc3a4 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 17 Nov 2025 10:34:43 +0100 Subject: ovl: drop ovl_setup_cred_for_create() It is now unused and can be removed. Link: https://patch.msgid.link/20251117-work-ovl-cred-guard-prepare-v2-6-bd1c97a36d7b@kernel.org Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 6cfa7857b352..0f01e005b915 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -617,39 +617,6 @@ DEFINE_CLASS(ovl_override_creator_creds, ovl_override_creator_creds(dentry, inode, mode), struct dentry *dentry, struct inode *inode, umode_t mode) -static const __maybe_unused struct cred *ovl_setup_cred_for_create(struct dentry *dentry, - struct inode *inode, - umode_t mode, - const struct cred *old_cred) -{ - int err; - struct cred *override_cred; - - override_cred = prepare_creds(); - if (!override_cred) - return ERR_PTR(-ENOMEM); - - override_cred->fsuid = inode->i_uid; - override_cred->fsgid = inode->i_gid; - err = security_dentry_create_files_as(dentry, mode, &dentry->d_name, - old_cred, override_cred); - if (err) { - put_cred(override_cred); - return ERR_PTR(err); - } - - /* - * Caller is going to match this with revert_creds() and drop - * referenec on the returned creds. - * We must be called with creator creds already, otherwise we risk - * leaking creds. - */ - old_cred = override_creds(override_cred); - WARN_ON_ONCE(old_cred != ovl_creds(dentry->d_sb)); - - return override_cred; -} - static int ovl_create_handle_whiteouts(struct dentry *dentry, struct inode *inode, struct ovl_cattr *attr) -- cgit v1.2.3