[PATCH v2 0/6] ovl: convert creation credential override to cred guard

[PATCH v2 0/6] ovl: convert creation credential override to cred guard

[Christian Brauner]

Hey,

This is on top of the overlayfs cleanup guard work I already sent out.
This cleans up the creation specific credential override.

The current code to override credentials for creation operations is
pretty difficult to understand as we 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_creds() because it would be an idempotent
revert. That elision doesn't buy us anything anymore though because it's
all reference count less anyway.

The fact that this is done in a function and that the revert is
happening in the original override makes this a lot to grasp.

By introducing a cleanup guard for the creation case we can make this a
lot easier to understand and extremely visually prevalent:

with_ovl_creds(dentry->d_sb) {
	scoped_class(prepare_creds_ovl, cred, dentry, inode, mode) {
		if (IS_ERR(cred))
			return PTR_ERR(cred);

		ovl_path_upper(dentry->d_parent, &realparentpath);

		/* more stuff you want to do */
}

I think this is a big improvement over what we have now.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Changes in v2:
- Rename the creation credential guard to avoid theoretical confusion
  with the removal of the ovl_revert_creds() function.
- Link to v1: https://patch.msgid.link/20251114-work-ovl-cred-guard-prepare-v1-0-4fc1208afa3d@kernel.org

---
Christian Brauner (6):
      ovl: add ovl_override_creator_creds cred guard
      ovl: port ovl_create_tmpfile() to new ovl_override_creator_creds cleanup guard
      ovl: reflow ovl_create_or_link()
      ovl: mark ovl_setup_cred_for_create() as unused temporarily
      ovl: port ovl_create_or_link() to new ovl_override_creator_creds cleanup guard
      ovl: drop ovl_setup_cred_for_create()

 fs/overlayfs/dir.c | 147 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 78 insertions(+), 69 deletions(-)
---
base-commit: fc64e774b2606549fe236fbf93fa6287c93dbdaa
change-id: 20251114-work-ovl-cred-guard-prepare-53210e7e41f8

[PATCH v2 1/6] ovl: add ovl_override_creator_creds cred guard

[Christian Brauner]

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.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/dir.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 4fd6ddd6f4ef..3eb0bb0b8f3b 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -575,6 +575,42 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 	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,

-- 
2.47.3

[PATCH v2 2/6] ovl: port ovl_create_tmpfile() to new ovl_override_creator_creds cleanup guard

[Christian Brauner]

This clearly indicates the double-credential override and makes the code
a lot easier to grasp with one glance.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/dir.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 3eb0bb0b8f3b..dad818de4386 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1387,7 +1387,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;
@@ -1396,10 +1395,10 @@ 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);
+	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,
@@ -1425,6 +1424,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 				ovl_file_free(of);
 			}
 		}
+	}
 	return err;
 }
 

-- 
2.47.3

[PATCH v2 3/6] ovl: reflow ovl_create_or_link()

[Christian Brauner]

Reflow the creation routine in preparation of porting it to a guard.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/dir.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index dad818de4386..150d2ae8e571 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -644,6 +644,15 @@ static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry,
 	return override_cred;
 }
 
+static int do_ovl_create_or_link(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)
 {
@@ -662,7 +671,6 @@ 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
@@ -676,16 +684,15 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 		 * create a new inode, so just use the ovl mounter's
 		 * fs{u,g}id.
 		 */
+
+		if (attr->hardlink)
+			return do_ovl_create_or_link(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);
-		}
-
-		if (!ovl_dentry_is_whiteout(dentry))
-			return ovl_create_upper(dentry, inode, attr);
-
-		return ovl_create_over_whiteout(dentry, inode, attr);
 
+		return do_ovl_create_or_link(dentry, inode, attr);
 	}
 	return err;
 }

-- 
2.47.3

[PATCH v2 4/6] ovl: mark ovl_setup_cred_for_create() as unused temporarily

[Christian Brauner]

The function will become unused in the next patch.
We'll remove it in later patches to keep the diff legible.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 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 150d2ae8e571..1bb311a25303 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -611,7 +611,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)

-- 
2.47.3

[PATCH v2 5/6] ovl: port ovl_create_or_link() to new ovl_override_creator_creds cleanup guard

[Christian Brauner]

This clearly indicates the double-credential override and makes the code
a lot easier to grasp with one glance.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/dir.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1bb311a25303..cb474b649ed2 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -657,10 +657,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".
@@ -688,12 +687,12 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 		if (attr->hardlink)
 			return do_ovl_create_or_link(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);
-
+		scoped_class(ovl_override_creator_creds, cred, dentry, inode, attr->mode) {
+			if (IS_ERR(cred))
+				return PTR_ERR(cred);
 			return do_ovl_create_or_link(dentry, inode, attr);
 		}
+	}
 	return err;
 }
 

-- 
2.47.3

[PATCH v2 6/6] ovl: drop ovl_setup_cred_for_create()

[Christian Brauner]

It is now unused and can be removed.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/dir.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index cb474b649ed2..e428a2de59fc 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -611,39 +611,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 do_ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 				 struct ovl_cattr *attr)
 {

-- 
2.47.3

Re: [PATCH v2 2/6] ovl: port ovl_create_tmpfile() to new ovl_override_creator_creds cleanup guard

[Amir Goldstein]

On Mon, Nov 17, 2025 at 10:35 AM Christian Brauner <brauner@kernel.org> wrote:
>
> This clearly indicates the double-credential override and makes the code
> a lot easier to grasp with one glance.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/dir.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 3eb0bb0b8f3b..dad818de4386 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1387,7 +1387,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;
> @@ -1396,10 +1395,10 @@ 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);
> +       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,
> @@ -1425,6 +1424,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
>                                 ovl_file_free(of);
>                         }
>                 }
> +       }
>         return err;
>  }
>
>
> --
> 2.47.3
>

Re: [PATCH v2 3/6] ovl: reflow ovl_create_or_link()

[Amir Goldstein]

On Mon, Nov 17, 2025 at 10:35 AM Christian Brauner <brauner@kernel.org> wrote:
>
> Reflow the creation routine in preparation of porting it to a guard.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/dir.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index dad818de4386..150d2ae8e571 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -644,6 +644,15 @@ static const struct cred *ovl_setup_cred_for_create(struct dentry *dentry,
>         return override_cred;
>  }
>
> +static int do_ovl_create_or_link(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)
>  {
> @@ -662,7 +671,6 @@ 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
> @@ -676,16 +684,15 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
>                  * create a new inode, so just use the ovl mounter's
>                  * fs{u,g}id.
>                  */
> +
> +               if (attr->hardlink)
> +                       return do_ovl_create_or_link(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);
> -               }
> -
> -               if (!ovl_dentry_is_whiteout(dentry))
> -                       return ovl_create_upper(dentry, inode, attr);
> -
> -               return ovl_create_over_whiteout(dentry, inode, attr);
>
> +               return do_ovl_create_or_link(dentry, inode, attr);
>         }
>         return err;
>  }
>
> --
> 2.47.3
>

Re: [PATCH v2 5/6] ovl: port ovl_create_or_link() to new ovl_override_creator_creds cleanup guard

[Amir Goldstein]

On Mon, Nov 17, 2025 at 10:35 AM Christian Brauner <brauner@kernel.org> wrote:
>
> This clearly indicates the double-credential override and makes the code
> a lot easier to grasp with one glance.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/dir.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 1bb311a25303..cb474b649ed2 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -657,10 +657,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".
> @@ -688,12 +687,12 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
>                 if (attr->hardlink)
>                         return do_ovl_create_or_link(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);
> -
> +               scoped_class(ovl_override_creator_creds, cred, dentry, inode, attr->mode) {
> +                       if (IS_ERR(cred))
> +                               return PTR_ERR(cred);
>                         return do_ovl_create_or_link(dentry, inode, attr);
>                 }
> +       }
>         return err;
>  }
>
>
> --
> 2.47.3
>