[PATCH v3 00/42] ovl: convert to cred guard

[PATCH v3 00/42] ovl: convert to cred guard

[Christian Brauner]

This adds an overlayfs specific extension of the cred guard
infrastructure I introduced. This allows all of overlayfs to be ported
to cred guards. I refactored a few functions to reduce the scope of the
cred guard. I think this is beneficial as it's visually very easy to
grasp the scope in one go. Lightly tested.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Changes in v3:
- Drop assert.
- Fix ovl_rename() refactoring and split into two.
- EDITME: use bulletpoints and terse descriptions.
- Link to v2: https://patch.msgid.link/20251113-work-ovl-cred-guard-v2-0-c08940095e90@kernel.org

Changes in v2:
- Fixed ovl_lookup() refactoring.
- Various other fixes.
- Added vfs debug assert to detect double credential overrides.
- Link to v1: https://patch.msgid.link/20251113-work-ovl-cred-guard-v1-0-fa9887f17061@kernel.org

---
Amir Goldstein (1):
      ovl: refactor ovl_iterate() and port to cred guard

Christian Brauner (41):
      ovl: add override_creds cleanup guard extension for overlayfs
      ovl: port ovl_copy_up_flags() to cred guards
      ovl: port ovl_create_or_link() to cred guard
      ovl: port ovl_set_link_redirect() to cred guard
      ovl: port ovl_do_remove() to cred guard
      ovl: port ovl_create_tmpfile() to cred guard
      ovl: port ovl_open_realfile() to cred guard
      ovl: port ovl_llseek() to cred guard
      ovl: port ovl_fsync() to cred guard
      ovl: port ovl_fallocate() to cred guard
      ovl: port ovl_fadvise() to cred guard
      ovl: port ovl_flush() to cred guard
      ovl: port ovl_setattr() to cred guard
      ovl: port ovl_getattr() to cred guard
      ovl: port ovl_permission() to cred guard
      ovl: port ovl_get_link() to cred guard
      ovl: port do_ovl_get_acl() to cred guard
      ovl: port ovl_set_or_remove_acl() to cred guard
      ovl: port ovl_fiemap() to cred guard
      ovl: port ovl_fileattr_set() to cred guard
      ovl: port ovl_fileattr_get() to cred guard
      ovl: port ovl_maybe_validate_verity() to cred guard
      ovl: port ovl_maybe_lookup_lowerdata() to cred guard
      ovl: don't override credentials for ovl_check_whiteouts()
      ovl: port ovl_dir_llseek() to cred guard
      ovl: port ovl_check_empty_dir() to cred guard
      ovl: port ovl_nlink_start() to cred guard
      ovl: port ovl_nlink_end() to cred guard
      ovl: port ovl_xattr_set() to cred guard
      ovl: port ovl_xattr_get() to cred guard
      ovl: port ovl_listxattr() to cred guard
      ovl: introduce struct ovl_renamedata
      ovl: extract do_ovl_rename() helper function
      ovl: port ovl_rename() to cred guard
      ovl: port ovl_copyfile() to cred guard
      ovl: refactor ovl_lookup()
      ovl: port ovl_lookup() to cred guard
      ovl: port ovl_lower_positive() to cred guard
      ovl: refactor ovl_fill_super()
      ovl: port ovl_fill_super() to cred guard
      ovl: remove ovl_revert_creds()

 fs/overlayfs/copy_up.c   |   6 +-
 fs/overlayfs/dir.c       | 439 +++++++++++++++++++++++------------------------
 fs/overlayfs/file.c      | 101 +++++------
 fs/overlayfs/inode.c     | 120 ++++++-------
 fs/overlayfs/namei.c     | 402 +++++++++++++++++++++----------------------
 fs/overlayfs/overlayfs.h |   6 +-
 fs/overlayfs/readdir.c   |  86 ++++------
 fs/overlayfs/super.c     |  89 +++++-----
 fs/overlayfs/util.c      |  18 +-
 fs/overlayfs/xattrs.c    |  35 ++--
 10 files changed, 615 insertions(+), 687 deletions(-)
---
base-commit: 2902367e352af16cbed9c67ca9022b52a0b738e7
change-id: 20251112-work-ovl-cred-guard-20daabcbf8fa

[PATCH v3 01/42] ovl: add override_creds cleanup guard extension for overlayfs

[Christian Brauner]

Overlayfs plucks the relevant creds from the superblock. Extend the
override_creds cleanup class I added to override_creds_ovl which uses
the ovl_override_creds() function as initialization helper. Add
with_ovl_creds() based on this new class.

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

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index c8fd5951fc5e..eeace590ba57 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -439,6 +439,11 @@ struct dentry *ovl_workdir(struct dentry *dentry);
 const struct cred *ovl_override_creds(struct super_block *sb);
 void ovl_revert_creds(const struct cred *old_cred);
 
+EXTEND_CLASS(override_creds, _ovl, ovl_override_creds(sb), struct super_block *sb)
+
+#define with_ovl_creds(sb) \
+	scoped_class(override_creds_ovl, __UNIQUE_ID(label), sb)
+
 static inline const struct cred *ovl_creds(struct super_block *sb)
 {
 	return OVL_FS(sb)->creator_cred;

-- 
2.47.3

[PATCH v3 02/42] ovl: port ovl_copy_up_flags() to cred guards

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 604a82acd164..bb0231fc61fc 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1214,7 +1214,6 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 {
 	int err = 0;
-	const struct cred *old_cred;
 	bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);
 
 	/*
@@ -1234,7 +1233,6 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 	if (err)
 		return err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
 	while (!err) {
 		struct dentry *next;
 		struct dentry *parent = NULL;
@@ -1254,12 +1252,12 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 			next = parent;
 		}
 
+		with_ovl_creds(dentry->d_sb)
 			err = ovl_copy_up_one(parent, next, flags);
 
 		dput(parent);
 		dput(next);
 	}
-	ovl_revert_creds(old_cred);
 
 	return err;
 }

-- 
2.47.3

[PATCH v3 03/42] ovl: port ovl_create_or_link() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index a5e9ddf3023b..93b81d4b6fb1 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -612,11 +612,10 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 			      struct ovl_cattr *attr, bool origin)
 {
 	int err;
-	const struct cred *old_cred, *new_cred = NULL;
+	const struct cred *new_cred __free(put_cred) = NULL;
 	struct dentry *parent = dentry->d_parent;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-
+	scoped_class(override_creds_ovl, old_cred, dentry->d_sb) {
 		/*
 		 * When linking a file with copy up origin into a new parent, mark the
 		 * new parent dir "impure".
@@ -624,7 +623,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 		if (origin) {
 			err = ovl_set_impure(parent, ovl_dentry_upper(parent));
 			if (err)
-			goto out_revert_creds;
+				return err;
 		}
 
 		if (!attr->hardlink) {
@@ -641,23 +640,17 @@ 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.
 			 */
-		new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode,
-						     old_cred);
-		err = PTR_ERR(new_cred);
-		if (IS_ERR(new_cred)) {
-			new_cred = NULL;
-			goto out_revert_creds;
-		}
+			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))
-		err = ovl_create_upper(dentry, inode, attr);
-	else
-		err = ovl_create_over_whiteout(dentry, inode, attr);
+			return ovl_create_upper(dentry, inode, attr);
 
-out_revert_creds:
-	ovl_revert_creds(old_cred);
-	put_cred(new_cred);
+		return ovl_create_over_whiteout(dentry, inode, attr);
+
+	}
 	return err;
 }
 

-- 
2.47.3

[PATCH v3 04/42] ovl: port ovl_set_link_redirect() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 93b81d4b6fb1..63f2b3d07f54 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -726,14 +726,8 @@ static int ovl_symlink(struct mnt_idmap *idmap, struct inode *dir,
 
 static int ovl_set_link_redirect(struct dentry *dentry)
 {
-	const struct cred *old_cred;
-	int err;
-
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = ovl_set_redirect(dentry, false);
-	ovl_revert_creds(old_cred);
-
-	return err;
+	with_ovl_creds(dentry->d_sb)
+		return ovl_set_redirect(dentry, false);
 }
 
 static int ovl_link(struct dentry *old, struct inode *newdir,

-- 
2.47.3

[PATCH v3 05/42] ovl: port ovl_do_remove() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 63f2b3d07f54..1a801fa40dd1 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -903,7 +903,6 @@ static void ovl_drop_nlink(struct dentry *dentry)
 static int ovl_do_remove(struct dentry *dentry, bool is_dir)
 {
 	int err;
-	const struct cred *old_cred;
 	bool lower_positive = ovl_lower_positive(dentry);
 	LIST_HEAD(list);
 
@@ -922,12 +921,12 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir)
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb) {
 		if (!lower_positive)
 			err = ovl_remove_upper(dentry, is_dir, &list);
 		else
 			err = ovl_remove_and_whiteout(dentry, &list);
-	ovl_revert_creds(old_cred);
+	}
 	if (!err) {
 		if (is_dir)
 			clear_nlink(dentry->d_inode);

-- 
2.47.3

[PATCH v3 06/42] ovl: port ovl_create_tmpfile() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1a801fa40dd1..86b72bf87833 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1323,7 +1323,7 @@ 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 *old_cred, *new_cred = NULL;
+	const struct cred *new_cred __free(put_cred) = NULL;
 	struct path realparentpath;
 	struct file *realfile;
 	struct ovl_file *of;
@@ -1332,27 +1332,25 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 	int flags = file->f_flags | OVL_OPEN_FLAGS;
 	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	scoped_class(override_creds_ovl, old_cred, dentry->d_sb) {
 		new_cred = ovl_setup_cred_for_create(dentry, inode, mode, old_cred);
-	err = PTR_ERR(new_cred);
-	if (IS_ERR(new_cred)) {
-		new_cred = NULL;
-		goto out_revert_creds;
-	}
+		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());
+		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);
+		pr_debug("tmpfile/open(%pd2, 0%o) = %i\n",
+			 realparentpath.dentry, mode, err);
 		if (err)
-		goto out_revert_creds;
+			return err;
 
 		of = ovl_file_alloc(realfile);
 		if (!of) {
 			fput(realfile);
-		err = -ENOMEM;
-		goto out_revert_creds;
+			return -ENOMEM;
 		}
 
 		/* ovl_instantiate() consumes the newdentry reference on success */
@@ -1364,9 +1362,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 			dput(newdentry);
 			ovl_file_free(of);
 		}
-out_revert_creds:
-	ovl_revert_creds(old_cred);
-	put_cred(new_cred);
+	}
 	return err;
 }
 

-- 
2.47.3

[PATCH v3 07/42] ovl: port ovl_open_realfile() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 7ab2c9daffd0..1f606b62997b 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -31,7 +31,6 @@ static struct file *ovl_open_realfile(const struct file *file,
 	struct inode *inode = file_inode(file);
 	struct mnt_idmap *real_idmap;
 	struct file *realfile;
-	const struct cred *old_cred;
 	int flags = file->f_flags | OVL_OPEN_FLAGS;
 	int acc_mode = ACC_MODE(flags);
 	int err;
@@ -39,9 +38,10 @@ static struct file *ovl_open_realfile(const struct file *file,
 	if (flags & O_APPEND)
 		acc_mode |= MAY_APPEND;
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	with_ovl_creds(inode->i_sb) {
 		real_idmap = mnt_idmap(realpath->mnt);
-	err = inode_permission(real_idmap, realinode, MAY_OPEN | acc_mode);
+		err = inode_permission(real_idmap, realinode,
+				       MAY_OPEN | acc_mode);
 		if (err) {
 			realfile = ERR_PTR(err);
 		} else {
@@ -49,9 +49,10 @@ static struct file *ovl_open_realfile(const struct file *file,
 				flags &= ~O_NOATIME;
 
 			realfile = backing_file_open(file_user_path(file),
-					     flags, realpath, current_cred());
+						     flags, realpath,
+						     current_cred());
+		}
 	}
-	ovl_revert_creds(old_cred);
 
 	pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
 		 file, file, ovl_whatisit(inode, realinode), file->f_flags,

-- 
2.47.3

[PATCH v3 08/42] ovl: port ovl_llseek() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 1f606b62997b..e713f27d70aa 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -245,7 +245,6 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file_inode(file);
 	struct file *realfile;
-	const struct cred *old_cred;
 	loff_t ret;
 
 	/*
@@ -274,9 +273,8 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence)
 	ovl_inode_lock(inode);
 	realfile->f_pos = file->f_pos;
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	with_ovl_creds(inode->i_sb)
 		ret = vfs_llseek(realfile, offset, whence);
-	ovl_revert_creds(old_cred);
 
 	file->f_pos = realfile->f_pos;
 	ovl_inode_unlock(inode);

-- 
2.47.3

[PATCH v3 09/42] ovl: port ovl_fsync() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index e713f27d70aa..6c5aa74f63ec 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -446,7 +446,6 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 	enum ovl_path_type type;
 	struct path upperpath;
 	struct file *upperfile;
-	const struct cred *old_cred;
 	int ret;
 
 	ret = ovl_sync_status(OVL_FS(file_inode(file)->i_sb));
@@ -463,11 +462,8 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 	if (IS_ERR(upperfile))
 		return PTR_ERR(upperfile);
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
-	ret = vfs_fsync_range(upperfile, start, end, datasync);
-	ovl_revert_creds(old_cred);
-
-	return ret;
+	with_ovl_creds(file_inode(file)->i_sb)
+		return vfs_fsync_range(upperfile, start, end, datasync);
 }
 
 static int ovl_mmap(struct file *file, struct vm_area_struct *vma)

-- 
2.47.3

[PATCH v3 10/42] ovl: port ovl_fallocate() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 6c5aa74f63ec..28263ad00dee 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -481,7 +481,6 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 {
 	struct inode *inode = file_inode(file);
 	struct file *realfile;
-	const struct cred *old_cred;
 	int ret;
 
 	inode_lock(inode);
@@ -496,9 +495,8 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 	if (IS_ERR(realfile))
 		goto out_unlock;
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
+	with_ovl_creds(inode->i_sb)
 		ret = vfs_fallocate(realfile, mode, offset, len);
-	ovl_revert_creds(old_cred);
 
 	/* Update size */
 	ovl_file_modified(file);

-- 
2.47.3

[PATCH v3 11/42] ovl: port ovl_fadvise() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 28263ad00dee..f562f908f48a 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -510,18 +510,13 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
 {
 	struct file *realfile;
-	const struct cred *old_cred;
-	int ret;
 
 	realfile = ovl_real_file(file);
 	if (IS_ERR(realfile))
 		return PTR_ERR(realfile);
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
-	ret = vfs_fadvise(realfile, offset, len, advice);
-	ovl_revert_creds(old_cred);
-
-	return ret;
+	with_ovl_creds(file_inode(file)->i_sb)
+		return vfs_fadvise(realfile, offset, len, advice);
 }
 
 enum ovl_copyop {

-- 
2.47.3

[PATCH v3 12/42] ovl: port ovl_flush() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index f562f908f48a..e375c7306051 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -620,7 +620,6 @@ static loff_t ovl_remap_file_range(struct file *file_in, loff_t pos_in,
 static int ovl_flush(struct file *file, fl_owner_t id)
 {
 	struct file *realfile;
-	const struct cred *old_cred;
 	int err = 0;
 
 	realfile = ovl_real_file(file);
@@ -628,9 +627,8 @@ static int ovl_flush(struct file *file, fl_owner_t id)
 		return PTR_ERR(realfile);
 
 	if (realfile->f_op->flush) {
-		old_cred = ovl_override_creds(file_inode(file)->i_sb);
+		with_ovl_creds(file_inode(file)->i_sb)
 			err = realfile->f_op->flush(realfile, id);
-		ovl_revert_creds(old_cred);
 	}
 
 	return err;

-- 
2.47.3

[PATCH v3 13/42] ovl: port ovl_setattr() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index e11f310ce092..7b28318b7f31 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -25,7 +25,6 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	bool full_copy_up = false;
 	struct dentry *upperdentry;
-	const struct cred *old_cred;
 
 	err = setattr_prepare(&nop_mnt_idmap, dentry, attr);
 	if (err)
@@ -78,9 +77,8 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			goto out_put_write;
 
 		inode_lock(upperdentry->d_inode);
-		old_cred = ovl_override_creds(dentry->d_sb);
+		with_ovl_creds(dentry->d_sb)
 			err = ovl_do_notify_change(ofs, upperdentry, attr);
-		ovl_revert_creds(old_cred);
 		if (!err)
 			ovl_copyattr(dentry->d_inode);
 		inode_unlock(upperdentry->d_inode);

-- 
2.47.3

[PATCH v3 14/42] ovl: port ovl_getattr() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 7b28318b7f31..00e1a47116d4 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -151,13 +151,22 @@ static void ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid)
 	}
 }
 
+static inline int ovl_real_getattr_nosec(struct super_block *sb,
+					 const struct path *path,
+					 struct kstat *stat, u32 request_mask,
+					 unsigned int flags)
+{
+	with_ovl_creds(sb)
+		return vfs_getattr_nosec(path, stat, request_mask, flags);
+}
+
 int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 		struct kstat *stat, u32 request_mask, unsigned int flags)
 {
 	struct dentry *dentry = path->dentry;
+	struct super_block *sb = dentry->d_sb;
 	enum ovl_path_type type;
 	struct path realpath;
-	const struct cred *old_cred;
 	struct inode *inode = d_inode(dentry);
 	bool is_dir = S_ISDIR(inode->i_mode);
 	int fsid = 0;
@@ -167,10 +176,9 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 	metacopy_blocks = ovl_is_metacopy_dentry(dentry);
 
 	type = ovl_path_real(dentry, &realpath);
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = vfs_getattr_nosec(&realpath, stat, request_mask, flags);
+	err = ovl_real_getattr_nosec(sb, &realpath, stat, request_mask, flags);
 	if (err)
-		goto out;
+		return err;
 
 	/* Report the effective immutable/append-only STATX flags */
 	generic_fill_statx_attr(inode, stat);
@@ -193,10 +201,10 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 					(!is_dir ? STATX_NLINK : 0);
 
 			ovl_path_lower(dentry, &realpath);
-			err = vfs_getattr_nosec(&realpath, &lowerstat, lowermask,
-						flags);
+			err = ovl_real_getattr_nosec(sb, &realpath, &lowerstat,
+						     lowermask, flags);
 			if (err)
-				goto out;
+				return err;
 
 			/*
 			 * Lower hardlinks may be broken on copy up to different
@@ -246,10 +254,11 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 
 			ovl_path_lowerdata(dentry, &realpath);
 			if (realpath.dentry) {
-				err = vfs_getattr_nosec(&realpath, &lowerdatastat,
+				err = ovl_real_getattr_nosec(sb, &realpath,
+							     &lowerdatastat,
 							     lowermask, flags);
 				if (err)
-					goto out;
+					return err;
 			} else {
 				lowerdatastat.blocks =
 					round_up(stat->size, stat->blksize) >> 9;
@@ -277,9 +286,6 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 	if (!is_dir && ovl_test_flag(OVL_INDEX, d_inode(dentry)))
 		stat->nlink = dentry->d_inode->i_nlink;
 
-out:
-	ovl_revert_creds(old_cred);
-
 	return err;
 }
 

-- 
2.47.3

[PATCH v3 15/42] ovl: port ovl_permission() to cred guard

[Christian Brauner]

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 00e1a47116d4..dca96db19f81 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -295,7 +295,6 @@ int ovl_permission(struct mnt_idmap *idmap,
 	struct inode *upperinode = ovl_inode_upper(inode);
 	struct inode *realinode;
 	struct path realpath;
-	const struct cred *old_cred;
 	int err;
 
 	/* Careful in RCU walk mode */
@@ -313,17 +312,15 @@ int ovl_permission(struct mnt_idmap *idmap,
 	if (err)
 		return err;
 
-	old_cred = ovl_override_creds(inode->i_sb);
 	if (!upperinode &&
 	    !special_file(realinode->i_mode) && mask & MAY_WRITE) {
 		mask &= ~(MAY_WRITE | MAY_APPEND);
 		/* Make sure mounter can read file for copy up later */
 		mask |= MAY_READ;
 	}
-	err = inode_permission(mnt_idmap(realpath.mnt), realinode, mask);
-	ovl_revert_creds(old_cred);
 
-	return err;
+	with_ovl_creds(inode->i_sb)
+		return inode_permission(mnt_idmap(realpath.mnt), realinode, mask);
 }
 
 static const char *ovl_get_link(struct dentry *dentry,

-- 
2.47.3

[PATCH v3 16/42] ovl: port ovl_get_link() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index dca96db19f81..3a35f9b125f4 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -327,16 +327,11 @@ static const char *ovl_get_link(struct dentry *dentry,
 				struct inode *inode,
 				struct delayed_call *done)
 {
-	const struct cred *old_cred;
-	const char *p;
-
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-	p = vfs_get_link(ovl_dentry_real(dentry), done);
-	ovl_revert_creds(old_cred);
-	return p;
+	with_ovl_creds(dentry->d_sb)
+		return vfs_get_link(ovl_dentry_real(dentry), done);
 }
 
 #ifdef CONFIG_FS_POSIX_ACL

-- 
2.47.3

[PATCH v3 17/42] ovl: port do_ovl_get_acl() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 3a35f9b125f4..1e74b3d9b7f3 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -461,11 +461,8 @@ struct posix_acl *do_ovl_get_acl(struct mnt_idmap *idmap,
 
 		acl = get_cached_acl_rcu(realinode, type);
 	} else {
-		const struct cred *old_cred;
-
-		old_cred = ovl_override_creds(inode->i_sb);
+		with_ovl_creds(inode->i_sb)
 			acl = ovl_get_acl_path(&realpath, posix_acl_xattr_name(type), noperm);
-		ovl_revert_creds(old_cred);
 	}
 
 	return acl;

-- 
2.47.3

[PATCH v3 18/42] ovl: port ovl_set_or_remove_acl() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 1e74b3d9b7f3..e6d6cfd9335d 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -474,7 +474,6 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 	int err;
 	struct path realpath;
 	const char *acl_name;
-	const struct cred *old_cred;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct dentry *upperdentry = ovl_dentry_upper(dentry);
 	struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
@@ -488,10 +487,8 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 		struct posix_acl *real_acl;
 
 		ovl_path_lower(dentry, &realpath);
-		old_cred = ovl_override_creds(dentry->d_sb);
-		real_acl = vfs_get_acl(mnt_idmap(realpath.mnt), realdentry,
-				       acl_name);
-		ovl_revert_creds(old_cred);
+		with_ovl_creds(dentry->d_sb)
+			real_acl = vfs_get_acl(mnt_idmap(realpath.mnt), realdentry, acl_name);
 		if (IS_ERR(real_acl)) {
 			err = PTR_ERR(real_acl);
 			goto out;
@@ -511,12 +508,12 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb) {
 		if (acl)
 			err = ovl_do_set_acl(ofs, realdentry, acl_name, acl);
 		else
 			err = ovl_do_remove_acl(ofs, realdentry, acl_name);
-	ovl_revert_creds(old_cred);
+	}
 	ovl_drop_write(dentry);
 
 	/* copy c/mtime */

-- 
2.47.3

[PATCH v3 19/42] ovl: port ovl_fiemap() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index e6d6cfd9335d..5574ce30e0b2 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -578,9 +578,7 @@ int ovl_update_time(struct inode *inode, int flags)
 static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		      u64 start, u64 len)
 {
-	int err;
 	struct inode *realinode = ovl_inode_realdata(inode);
-	const struct cred *old_cred;
 
 	if (!realinode)
 		return -EIO;
@@ -588,11 +586,8 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (!realinode->i_op->fiemap)
 		return -EOPNOTSUPP;
 
-	old_cred = ovl_override_creds(inode->i_sb);
-	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
-	ovl_revert_creds(old_cred);
-
-	return err;
+	with_ovl_creds(inode->i_sb)
+		return realinode->i_op->fiemap(realinode, fieinfo, start, len);
 }
 
 /*

-- 
2.47.3

[PATCH v3 20/42] ovl: port ovl_fileattr_set() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 5574ce30e0b2..3a23eb038097 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -638,7 +638,6 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 {
 	struct inode *inode = d_inode(dentry);
 	struct path upperpath;
-	const struct cred *old_cred;
 	unsigned int flags;
 	int err;
 
@@ -650,7 +649,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 		if (err)
 			goto out;
 
-		old_cred = ovl_override_creds(inode->i_sb);
+		with_ovl_creds(inode->i_sb) {
 			/*
 			 * Store immutable/append-only flags in xattr and clear them
 			 * in upper fileattr (in case they were set by older kernel)
@@ -661,7 +660,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 			err = ovl_set_protattr(inode, upperpath.dentry, fa);
 			if (!err)
 				err = ovl_real_fileattr_set(&upperpath, fa);
-		ovl_revert_creds(old_cred);
+		}
 		ovl_drop_write(dentry);
 
 		/*

-- 
2.47.3

[PATCH v3 21/42] ovl: port ovl_fileattr_get() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 3a23eb038097..40671fcc6c4e 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -714,15 +714,13 @@ int ovl_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
 {
 	struct inode *inode = d_inode(dentry);
 	struct path realpath;
-	const struct cred *old_cred;
 	int err;
 
 	ovl_path_real(dentry, &realpath);
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	with_ovl_creds(inode->i_sb)
 		err = ovl_real_fileattr_get(&realpath, fa);
 	ovl_fileattr_prot_flags(inode, fa);
-	ovl_revert_creds(old_cred);
 
 	return err;
 }

-- 
2.47.3

[PATCH v3 22/42] ovl: port ovl_maybe_validate_verity() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index e93bcc5727bc..dbacf02423cb 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -979,15 +979,10 @@ static int ovl_maybe_validate_verity(struct dentry *dentry)
 		return err;
 
 	if (!ovl_test_flag(OVL_VERIFIED_DIGEST, inode)) {
-		const struct cred *old_cred;
-
-		old_cred = ovl_override_creds(dentry->d_sb);
-
+		with_ovl_creds(dentry->d_sb)
 			err = ovl_validate_verity(ofs, &metapath, &datapath);
 		if (err == 0)
 			ovl_set_flag(OVL_VERIFIED_DIGEST, inode);
-
-		ovl_revert_creds(old_cred);
 	}
 
 	ovl_inode_unlock(inode);

-- 
2.47.3

[PATCH v3 23/42] ovl: port ovl_maybe_lookup_lowerdata() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index dbacf02423cb..49874525cf52 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -996,7 +996,6 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 	struct inode *inode = d_inode(dentry);
 	const char *redirect = ovl_lowerdata_redirect(inode);
 	struct ovl_path datapath = {};
-	const struct cred *old_cred;
 	int err;
 
 	if (!redirect || ovl_dentry_lowerdata(dentry))
@@ -1014,9 +1013,8 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 	if (ovl_dentry_lowerdata(dentry))
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb)
 		err = ovl_lookup_data_layers(dentry, redirect, &datapath);
-	ovl_revert_creds(old_cred);
 	if (err)
 		goto out_err;
 

-- 
2.47.3

[PATCH v3 24/42] ovl: don't override credentials for ovl_check_whiteouts()

[Christian Brauner]

The function is only called when rdd->dentry is non-NULL:

if (!err && rdd->first_maybe_whiteout && rdd->dentry)
    err = ovl_check_whiteouts(realpath, rdd);

| Caller                        | Sets rdd->dentry? | Can call ovl_check_whiteouts()? |
|-------------------------------|-------------------|---------------------------------|
| ovl_dir_read_merged()         | ✓ Yes (line 430)  | ✓ YES                           |
| ovl_dir_read_impure()         | ✗ No              | ✗ NO                            |
| ovl_check_d_type_supported()  | ✗ No              | ✗ NO                            |
| ovl_workdir_cleanup_recurse() | ✗ No              | ✗ NO                            |
| ovl_indexdir_cleanup()        | ✗ No              | ✗ NO                            |

VFS layer (.iterate_shared file operation)
  → ovl_iterate()
      [CRED OVERRIDE]
      → ovl_cache_get()
          → ovl_dir_read_merged()
              → ovl_dir_read()
                  → ovl_check_whiteouts()
      [CRED REVERT]

ovl_unlink()
  → ovl_do_remove()
      → ovl_check_empty_dir()
          [CRED OVERRIDE]
          → ovl_dir_read_merged()
              → ovl_dir_read()
                  → ovl_check_whiteouts()
          [CRED REVERT]

ovl_rename()
  → ovl_check_empty_dir()
      [CRED OVERRIDE]
      → ovl_dir_read_merged()
          → ovl_dir_read()
              → ovl_check_whiteouts()
      [CRED REVERT]

All valid callchains already override credentials so drop the override.

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

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 1e9792cc557b..12f0bb1480d7 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -348,11 +348,7 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
 
 static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data *rdd)
 {
-	int err = 0;
 	struct dentry *dentry, *dir = path->dentry;
-	const struct cred *old_cred;
-
-	old_cred = ovl_override_creds(rdd->dentry->d_sb);
 
 	while (rdd->first_maybe_whiteout) {
 		struct ovl_cache_entry *p =
@@ -365,13 +361,11 @@ static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data
 			p->is_whiteout = ovl_is_whiteout(dentry);
 			dput(dentry);
 		} else if (PTR_ERR(dentry) == -EINTR) {
-			err = -EINTR;
-			break;
+			return -EINTR;
 		}
 	}
-	ovl_revert_creds(old_cred);
 
-	return err;
+	return 0;
 }
 
 static inline int ovl_dir_read(const struct path *realpath,

-- 
2.47.3

[PATCH v3 25/42] ovl: refactor ovl_iterate() and port to cred guard

[Christian Brauner]

From: Amir Goldstein <amir73il@gmail.com>

factor out ovl_iterate_merged() and move some code into
ovl_iterate_real() for easier use of the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 12f0bb1480d7..dd76186ae739 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -804,6 +804,18 @@ static int ovl_iterate_real(struct file *file, struct dir_context *ctx)
 		.xinowarn = ovl_xino_warn(ofs),
 	};
 
+	/*
+	 * With xino, we need to adjust d_ino of lower entries.
+	 * On same fs, if parent is merge, then need to adjust d_ino for '..',
+	 * and if dir is impure then need to adjust d_ino for copied up entries.
+	 * Otherwise, we can iterate the real dir directly.
+	 */
+	if (!ovl_xino_bits(ofs) &&
+	    !(ovl_same_fs(ofs) &&
+	      (ovl_is_impure_dir(file) ||
+	       OVL_TYPE_MERGE(ovl_path_type(dir->d_parent)))))
+		return iterate_dir(od->realfile, ctx);
+
 	if (rdt.xinobits && lower_layer)
 		rdt.fsid = lower_layer->fsid;
 
@@ -832,44 +844,20 @@ static int ovl_iterate_real(struct file *file, struct dir_context *ctx)
 	return err;
 }
 
-
-static int ovl_iterate(struct file *file, struct dir_context *ctx)
+static int ovl_iterate_merged(struct file *file, struct dir_context *ctx)
 {
 	struct ovl_dir_file *od = file->private_data;
 	struct dentry *dentry = file->f_path.dentry;
-	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct ovl_cache_entry *p;
-	const struct cred *old_cred;
 	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-	if (!ctx->pos)
-		ovl_dir_reset(file);
-
-	if (od->is_real) {
-		/*
-		 * If parent is merge, then need to adjust d_ino for '..', if
-		 * dir is impure then need to adjust d_ino for copied up
-		 * entries.
-		 */
-		if (ovl_xino_bits(ofs) ||
-		    (ovl_same_fs(ofs) &&
-		     (ovl_is_impure_dir(file) ||
-		      OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent))))) {
-			err = ovl_iterate_real(file, ctx);
-		} else {
-			err = iterate_dir(od->realfile, ctx);
-		}
-		goto out;
-	}
-
 	if (!od->cache) {
 		struct ovl_dir_cache *cache;
 
 		cache = ovl_cache_get(dentry);
 		err = PTR_ERR(cache);
 		if (IS_ERR(cache))
-			goto out;
+			return err;
 
 		od->cache = cache;
 		ovl_seek_cursor(od, ctx->pos);
@@ -881,7 +869,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
 			if (!p->ino || p->check_xwhiteout) {
 				err = ovl_cache_update(&file->f_path, p, !p->ino);
 				if (err)
-					goto out;
+					return err;
 			}
 		}
 		/* ovl_cache_update() sets is_whiteout on stale entry */
@@ -892,12 +880,24 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
 		od->cursor = p->l_node.next;
 		ctx->pos++;
 	}
-	err = 0;
-out:
-	ovl_revert_creds(old_cred);
 	return err;
 }
 
+static int ovl_iterate(struct file *file, struct dir_context *ctx)
+{
+	struct ovl_dir_file *od = file->private_data;
+
+	if (!ctx->pos)
+		ovl_dir_reset(file);
+
+	with_ovl_creds(file_dentry(file)->d_sb) {
+		if (od->is_real)
+			return ovl_iterate_real(file, ctx);
+
+		return ovl_iterate_merged(file, ctx);
+	}
+}
+
 static loff_t ovl_dir_llseek(struct file *file, loff_t offset, int origin)
 {
 	loff_t res;

-- 
2.47.3

[PATCH v3 26/42] ovl: port ovl_dir_llseek() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index dd76186ae739..f3bdc080ca85 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -941,14 +941,8 @@ static loff_t ovl_dir_llseek(struct file *file, loff_t offset, int origin)
 static struct file *ovl_dir_open_realfile(const struct file *file,
 					  const struct path *realpath)
 {
-	struct file *res;
-	const struct cred *old_cred;
-
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
-	res = ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE));
-	ovl_revert_creds(old_cred);
-
-	return res;
+	with_ovl_creds(file_inode(file)->i_sb)
+		return ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE));
 }
 
 /*

-- 
2.47.3

[PATCH v3 27/42] ovl: port ovl_check_empty_dir() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index f3bdc080ca85..01d4a70eb395 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -1063,11 +1063,9 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
 	int err;
 	struct ovl_cache_entry *p, *n;
 	struct rb_root root = RB_ROOT;
-	const struct cred *old_cred;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb)
 		err = ovl_dir_read_merged(dentry, list, &root);
-	ovl_revert_creds(old_cred);
 	if (err)
 		return err;
 

-- 
2.47.3

[PATCH v3 28/42] ovl: port ovl_nlink_start() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f76672f2e686..2280980cb3c3 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1147,7 +1147,6 @@ static void ovl_cleanup_index(struct dentry *dentry)
 int ovl_nlink_start(struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
-	const struct cred *old_cred;
 	int err;
 
 	if (WARN_ON(!inode))
@@ -1184,15 +1183,14 @@ int ovl_nlink_start(struct dentry *dentry)
 	if (d_is_dir(dentry) || !ovl_test_flag(OVL_INDEX, inode))
 		return 0;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
 	/*
 	 * The overlay inode nlink should be incremented/decremented IFF the
 	 * upper operation succeeds, along with nlink change of upper inode.
 	 * Therefore, before link/unlink/rename, we store the union nlink
 	 * value relative to the upper inode nlink in an upper inode xattr.
 	 */
+	with_ovl_creds(dentry->d_sb)
 		err = ovl_set_nlink_upper(dentry);
-	ovl_revert_creds(old_cred);
 	if (err)
 		goto out_drop_write;
 

-- 
2.47.3

[PATCH v3 29/42] ovl: port ovl_nlink_end() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 2280980cb3c3..e2f2e0d17f0b 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1211,11 +1211,8 @@ void ovl_nlink_end(struct dentry *dentry)
 	ovl_drop_write(dentry);
 
 	if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) {
-		const struct cred *old_cred;
-
-		old_cred = ovl_override_creds(dentry->d_sb);
+		with_ovl_creds(dentry->d_sb)
 			ovl_cleanup_index(dentry);
-		ovl_revert_creds(old_cred);
 	}
 
 	ovl_inode_unlock(inode);

-- 
2.47.3

[PATCH v3 30/42] ovl: port ovl_xattr_set() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c
index 88055deca936..787df86acb26 100644
--- a/fs/overlayfs/xattrs.c
+++ b/fs/overlayfs/xattrs.c
@@ -41,13 +41,11 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 	struct dentry *upperdentry = ovl_i_dentry_upper(inode);
 	struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
 	struct path realpath;
-	const struct cred *old_cred;
 
 	if (!value && !upperdentry) {
 		ovl_path_lower(dentry, &realpath);
-		old_cred = ovl_override_creds(dentry->d_sb);
+		with_ovl_creds(dentry->d_sb)
 			err = vfs_getxattr(mnt_idmap(realpath.mnt), realdentry, name, NULL, 0);
-		ovl_revert_creds(old_cred);
 		if (err < 0)
 			goto out;
 	}
@@ -64,15 +62,14 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb) {
 		if (value) {
-		err = ovl_do_setxattr(ofs, realdentry, name, value, size,
-				      flags);
+			err = ovl_do_setxattr(ofs, realdentry, name, value, size, flags);
 		} else {
 			WARN_ON(flags != XATTR_REPLACE);
 			err = ovl_do_removexattr(ofs, realdentry, name);
 		}
-	ovl_revert_creds(old_cred);
+	}
 	ovl_drop_write(dentry);
 
 	/* copy c/mtime */

-- 
2.47.3

[PATCH v3 31/42] ovl: port ovl_xattr_get() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c
index 787df86acb26..788182fff3e0 100644
--- a/fs/overlayfs/xattrs.c
+++ b/fs/overlayfs/xattrs.c
@@ -81,15 +81,11 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char *name,
 			 void *value, size_t size)
 {
-	ssize_t res;
-	const struct cred *old_cred;
 	struct path realpath;
 
 	ovl_i_path_real(inode, &realpath);
-	old_cred = ovl_override_creds(dentry->d_sb);
-	res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size);
-	ovl_revert_creds(old_cred);
-	return res;
+	with_ovl_creds(dentry->d_sb)
+		return vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size);
 }
 
 static bool ovl_can_list(struct super_block *sb, const char *s)

-- 
2.47.3

[PATCH v3 32/42] ovl: port ovl_listxattr() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c
index 788182fff3e0..aa95855c7023 100644
--- a/fs/overlayfs/xattrs.c
+++ b/fs/overlayfs/xattrs.c
@@ -109,12 +109,10 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 	ssize_t res;
 	size_t len;
 	char *s;
-	const struct cred *old_cred;
 	size_t prefix_len, name_len;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb)
 		res = vfs_listxattr(realdentry, list, size);
-	ovl_revert_creds(old_cred);
 	if (res <= 0 || size == 0)
 		return res;
 

-- 
2.47.3

[PATCH v3 33/42] ovl: introduce struct ovl_renamedata

[Christian Brauner]

Add a struct ovl_renamedata to group rename-related state that was
previously stored in local variables. Embedd struct renamedata directly
aligning with the vfs.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 86b72bf87833..052929b9b99d 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1090,6 +1090,15 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
 	return err;
 }
 
+struct ovl_renamedata {
+	struct renamedata;
+	struct dentry *opaquedir;
+	struct dentry *olddentry;
+	struct dentry *newdentry;
+	bool cleanup_whiteout;
+	bool overwrite;
+};
+
 static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		      struct dentry *old, struct inode *newdir,
 		      struct dentry *new, unsigned int flags)
@@ -1097,53 +1106,57 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	int err;
 	struct dentry *old_upperdir;
 	struct dentry *new_upperdir;
-	struct dentry *olddentry = NULL;
-	struct dentry *newdentry = NULL;
 	struct dentry *trap, *de;
 	bool old_opaque;
 	bool new_opaque;
-	bool cleanup_whiteout = false;
 	bool update_nlink = false;
-	bool overwrite = !(flags & RENAME_EXCHANGE);
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	bool samedir = olddir == newdir;
-	struct dentry *opaquedir = NULL;
 	const struct cred *old_cred = NULL;
 	struct ovl_fs *ofs = OVL_FS(old->d_sb);
+	struct ovl_renamedata ovlrd = {
+		.old_parent		= old->d_parent,
+		.old_dentry		= old,
+		.new_parent		= new->d_parent,
+		.new_dentry		= new,
+		.flags			= flags,
+		.cleanup_whiteout	= false,
+		.overwrite		= !(flags & RENAME_EXCHANGE),
+	};
 	LIST_HEAD(list);
+	bool samedir = ovlrd.old_parent == ovlrd.new_parent;
 
 	err = -EINVAL;
-	if (flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
+	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
 		goto out;
 
-	flags &= ~RENAME_NOREPLACE;
+	ovlrd.flags &= ~RENAME_NOREPLACE;
 
 	/* Don't copy up directory trees */
 	err = -EXDEV;
 	if (!ovl_can_move(old))
 		goto out;
-	if (!overwrite && !ovl_can_move(new))
+	if (!ovlrd.overwrite && !ovl_can_move(new))
 		goto out;
 
-	if (overwrite && new_is_dir && !ovl_pure_upper(new)) {
+	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
 		err = ovl_check_empty_dir(new, &list);
 		if (err)
 			goto out;
 	}
 
-	if (overwrite) {
+	if (ovlrd.overwrite) {
 		if (ovl_lower_positive(old)) {
 			if (!ovl_dentry_is_whiteout(new)) {
 				/* Whiteout source */
-				flags |= RENAME_WHITEOUT;
+				ovlrd.flags |= RENAME_WHITEOUT;
 			} else {
 				/* Switch whiteouts */
-				flags |= RENAME_EXCHANGE;
+				ovlrd.flags |= RENAME_EXCHANGE;
 			}
 		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
-			flags |= RENAME_EXCHANGE;
-			cleanup_whiteout = true;
+			ovlrd.flags |= RENAME_EXCHANGE;
+			ovlrd.cleanup_whiteout = true;
 		}
 	}
 
@@ -1151,10 +1164,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (err)
 		goto out;
 
-	err = ovl_copy_up(new->d_parent);
+	err = ovl_copy_up(ovlrd.new_parent);
 	if (err)
 		goto out;
-	if (!overwrite) {
+	if (!ovlrd.overwrite) {
 		err = ovl_copy_up(new);
 		if (err)
 			goto out;
@@ -1176,16 +1189,16 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	old_cred = ovl_override_creds(old->d_sb);
 
 	if (!list_empty(&list)) {
-		opaquedir = ovl_clear_empty(new, &list);
-		err = PTR_ERR(opaquedir);
-		if (IS_ERR(opaquedir)) {
-			opaquedir = NULL;
+		ovlrd.opaquedir = ovl_clear_empty(new, &list);
+		err = PTR_ERR(ovlrd.opaquedir);
+		if (IS_ERR(ovlrd.opaquedir)) {
+			ovlrd.opaquedir = NULL;
 			goto out_revert_creds;
 		}
 	}
 
-	old_upperdir = ovl_dentry_upper(old->d_parent);
-	new_upperdir = ovl_dentry_upper(new->d_parent);
+	old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
+	new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
 
 	if (!samedir) {
 		/*
@@ -1195,12 +1208,12 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		 * lookup the origin inodes of the entries to fill d_ino.
 		 */
 		if (ovl_type_origin(old)) {
-			err = ovl_set_impure(new->d_parent, new_upperdir);
+			err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
 			if (err)
 				goto out_revert_creds;
 		}
-		if (!overwrite && ovl_type_origin(new)) {
-			err = ovl_set_impure(old->d_parent, old_upperdir);
+		if (!ovlrd.overwrite && ovl_type_origin(new)) {
+			err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
 			if (err)
 				goto out_revert_creds;
 		}
@@ -1217,10 +1230,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	olddentry = de;
+	ovlrd.olddentry = de;
 
 	err = -ESTALE;
-	if (!ovl_matches_upper(old, olddentry))
+	if (!ovl_matches_upper(old, ovlrd.olddentry))
 		goto out_unlock;
 
 	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
@@ -1228,73 +1241,73 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	newdentry = de;
+	ovlrd.newdentry = de;
 
 	old_opaque = ovl_dentry_is_opaque(old);
 	new_opaque = ovl_dentry_is_opaque(new);
 
 	err = -ESTALE;
 	if (d_inode(new) && ovl_dentry_upper(new)) {
-		if (opaquedir) {
-			if (newdentry != opaquedir)
+		if (ovlrd.opaquedir) {
+			if (ovlrd.newdentry != ovlrd.opaquedir)
 				goto out_unlock;
 		} else {
-			if (!ovl_matches_upper(new, newdentry))
+			if (!ovl_matches_upper(new, ovlrd.newdentry))
 				goto out_unlock;
 		}
 	} else {
-		if (!d_is_negative(newdentry)) {
-			if (!new_opaque || !ovl_upper_is_whiteout(ofs, newdentry))
+		if (!d_is_negative(ovlrd.newdentry)) {
+			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.newdentry))
 				goto out_unlock;
 		} else {
-			if (flags & RENAME_EXCHANGE)
+			if (ovlrd.flags & RENAME_EXCHANGE)
 				goto out_unlock;
 		}
 	}
 
-	if (olddentry == trap)
+	if (ovlrd.olddentry == trap)
 		goto out_unlock;
-	if (newdentry == trap)
+	if (ovlrd.newdentry == trap)
 		goto out_unlock;
 
-	if (olddentry->d_inode == newdentry->d_inode)
+	if (ovlrd.olddentry->d_inode == ovlrd.newdentry->d_inode)
 		goto out_unlock;
 
 	err = 0;
 	if (ovl_type_merge_or_lower(old))
 		err = ovl_set_redirect(old, samedir);
-	else if (is_dir && !old_opaque && ovl_type_merge(new->d_parent))
-		err = ovl_set_opaque_xerr(old, olddentry, -EXDEV);
+	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
+		err = ovl_set_opaque_xerr(old, ovlrd.olddentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	if (!overwrite && ovl_type_merge_or_lower(new))
+	if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
 		err = ovl_set_redirect(new, samedir);
-	else if (!overwrite && new_is_dir && !new_opaque &&
-		 ovl_type_merge(old->d_parent))
-		err = ovl_set_opaque_xerr(new, newdentry, -EXDEV);
+	else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
+		 ovl_type_merge(ovlrd.old_parent))
+		err = ovl_set_opaque_xerr(new, ovlrd.newdentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename(ofs, old_upperdir, olddentry,
-			    new_upperdir, newdentry, flags);
+	err = ovl_do_rename(ofs, old_upperdir, ovlrd.olddentry,
+			    new_upperdir, ovlrd.newdentry, flags);
 	unlock_rename(new_upperdir, old_upperdir);
 	if (err)
 		goto out_revert_creds;
 
-	if (cleanup_whiteout)
-		ovl_cleanup(ofs, old_upperdir, newdentry);
+	if (ovlrd.cleanup_whiteout)
+		ovl_cleanup(ofs, old_upperdir, ovlrd.newdentry);
 
-	if (overwrite && d_inode(new)) {
+	if (ovlrd.overwrite && d_inode(new)) {
 		if (new_is_dir)
 			clear_nlink(d_inode(new));
 		else
 			ovl_drop_nlink(new);
 	}
 
-	ovl_dir_modified(old->d_parent, ovl_type_origin(old) ||
-			 (!overwrite && ovl_type_origin(new)));
-	ovl_dir_modified(new->d_parent, ovl_type_origin(old) ||
+	ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
+			 (!ovlrd.overwrite && ovl_type_origin(new)));
+	ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
 			 (d_inode(new) && ovl_type_origin(new)));
 
 	/* copy ctime: */
@@ -1309,9 +1322,9 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	else
 		ovl_drop_write(old);
 out:
-	dput(newdentry);
-	dput(olddentry);
-	dput(opaquedir);
+	dput(ovlrd.newdentry);
+	dput(ovlrd.olddentry);
+	dput(ovlrd.opaquedir);
 	ovl_cache_free(&list);
 	return err;
 

-- 
2.47.3

[PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Christian Brauner]

Extract the code that runs under overridden credentials into a separate
do_ovl_rename() helper function. Error handling is simplified. The
helper returns errors directly instead of using goto labels.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 052929b9b99d..0812bb4ee4f6 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1099,107 +1099,28 @@ struct ovl_renamedata {
 	bool overwrite;
 };
 
-static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
-		      struct dentry *old, struct inode *newdir,
-		      struct dentry *new, unsigned int flags)
+static int do_ovl_rename(struct ovl_renamedata *ovlrd, struct list_head *list)
 {
-	int err;
-	struct dentry *old_upperdir;
-	struct dentry *new_upperdir;
-	struct dentry *trap, *de;
-	bool old_opaque;
-	bool new_opaque;
-	bool update_nlink = false;
+	struct dentry *old = ovlrd->old_dentry;
+	struct dentry *new = ovlrd->new_dentry;
+	struct ovl_fs *ofs = OVL_FS(old->d_sb);
+	unsigned int flags = ovlrd->flags;
+	struct dentry *old_upperdir = ovl_dentry_upper(ovlrd->old_parent);
+	struct dentry *new_upperdir = ovl_dentry_upper(ovlrd->new_parent);
+	bool samedir = ovlrd->old_parent == ovlrd->new_parent;
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	const struct cred *old_cred = NULL;
-	struct ovl_fs *ofs = OVL_FS(old->d_sb);
-	struct ovl_renamedata ovlrd = {
-		.old_parent		= old->d_parent,
-		.old_dentry		= old,
-		.new_parent		= new->d_parent,
-		.new_dentry		= new,
-		.flags			= flags,
-		.cleanup_whiteout	= false,
-		.overwrite		= !(flags & RENAME_EXCHANGE),
-	};
-	LIST_HEAD(list);
-	bool samedir = ovlrd.old_parent == ovlrd.new_parent;
-
-	err = -EINVAL;
-	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
-		goto out;
-
-	ovlrd.flags &= ~RENAME_NOREPLACE;
-
-	/* Don't copy up directory trees */
-	err = -EXDEV;
-	if (!ovl_can_move(old))
-		goto out;
-	if (!ovlrd.overwrite && !ovl_can_move(new))
-		goto out;
-
-	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
-		err = ovl_check_empty_dir(new, &list);
-		if (err)
-			goto out;
-	}
-
-	if (ovlrd.overwrite) {
-		if (ovl_lower_positive(old)) {
-			if (!ovl_dentry_is_whiteout(new)) {
-				/* Whiteout source */
-				ovlrd.flags |= RENAME_WHITEOUT;
-			} else {
-				/* Switch whiteouts */
-				ovlrd.flags |= RENAME_EXCHANGE;
-			}
-		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
-			ovlrd.flags |= RENAME_EXCHANGE;
-			ovlrd.cleanup_whiteout = true;
-		}
-	}
-
-	err = ovl_copy_up(old);
-	if (err)
-		goto out;
-
-	err = ovl_copy_up(ovlrd.new_parent);
-	if (err)
-		goto out;
-	if (!ovlrd.overwrite) {
-		err = ovl_copy_up(new);
-		if (err)
-			goto out;
-	} else if (d_inode(new)) {
-		err = ovl_nlink_start(new);
-		if (err)
-			goto out;
-
-		update_nlink = true;
-	}
-
-	if (!update_nlink) {
-		/* ovl_nlink_start() took ovl_want_write() */
-		err = ovl_want_write(old);
-		if (err)
-			goto out;
-	}
-
-	old_cred = ovl_override_creds(old->d_sb);
+	struct dentry *trap, *de;
+	bool old_opaque, new_opaque;
+	int err;
 
-	if (!list_empty(&list)) {
-		ovlrd.opaquedir = ovl_clear_empty(new, &list);
-		err = PTR_ERR(ovlrd.opaquedir);
-		if (IS_ERR(ovlrd.opaquedir)) {
-			ovlrd.opaquedir = NULL;
-			goto out_revert_creds;
-		}
+	if (!list_empty(list)) {
+		de = ovl_clear_empty(new, list);
+		if (IS_ERR(de))
+			return PTR_ERR(de);
+		ovlrd->opaquedir = de;
 	}
 
-	old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
-	new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
-
 	if (!samedir) {
 		/*
 		 * When moving a merge dir or non-dir with copy up origin into
@@ -1208,32 +1129,30 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		 * lookup the origin inodes of the entries to fill d_ino.
 		 */
 		if (ovl_type_origin(old)) {
-			err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
+			err = ovl_set_impure(ovlrd->new_parent, new_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
-		if (!ovlrd.overwrite && ovl_type_origin(new)) {
-			err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
+		if (!ovlrd->overwrite && ovl_type_origin(new)) {
+			err = ovl_set_impure(ovlrd->old_parent, old_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
 	}
 
 	trap = lock_rename(new_upperdir, old_upperdir);
-	if (IS_ERR(trap)) {
-		err = PTR_ERR(trap);
-		goto out_revert_creds;
-	}
+	if (IS_ERR(trap))
+		return PTR_ERR(trap);
 
 	de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir,
 			      old->d_name.len);
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.olddentry = de;
+	ovlrd->olddentry = de;
 
 	err = -ESTALE;
-	if (!ovl_matches_upper(old, ovlrd.olddentry))
+	if (!ovl_matches_upper(old, ovlrd->olddentry))
 		goto out_unlock;
 
 	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
@@ -1241,73 +1160,74 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.newdentry = de;
+	ovlrd->newdentry = de;
 
 	old_opaque = ovl_dentry_is_opaque(old);
 	new_opaque = ovl_dentry_is_opaque(new);
 
 	err = -ESTALE;
 	if (d_inode(new) && ovl_dentry_upper(new)) {
-		if (ovlrd.opaquedir) {
-			if (ovlrd.newdentry != ovlrd.opaquedir)
+		if (ovlrd->opaquedir) {
+			if (ovlrd->newdentry != ovlrd->opaquedir)
 				goto out_unlock;
 		} else {
-			if (!ovl_matches_upper(new, ovlrd.newdentry))
+			if (!ovl_matches_upper(new, ovlrd->newdentry))
 				goto out_unlock;
 		}
 	} else {
-		if (!d_is_negative(ovlrd.newdentry)) {
-			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.newdentry))
+		if (!d_is_negative(ovlrd->newdentry)) {
+			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd->newdentry))
 				goto out_unlock;
 		} else {
-			if (ovlrd.flags & RENAME_EXCHANGE)
+			if (flags & RENAME_EXCHANGE)
 				goto out_unlock;
 		}
 	}
 
-	if (ovlrd.olddentry == trap)
+	if (ovlrd->olddentry == trap)
 		goto out_unlock;
-	if (ovlrd.newdentry == trap)
+	if (ovlrd->newdentry == trap)
 		goto out_unlock;
 
-	if (ovlrd.olddentry->d_inode == ovlrd.newdentry->d_inode)
+	if (ovlrd->olddentry->d_inode == ovlrd->newdentry->d_inode)
 		goto out_unlock;
 
 	err = 0;
 	if (ovl_type_merge_or_lower(old))
 		err = ovl_set_redirect(old, samedir);
-	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
-		err = ovl_set_opaque_xerr(old, ovlrd.olddentry, -EXDEV);
+	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd->new_parent))
+		err = ovl_set_opaque_xerr(old, ovlrd->olddentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
+	if (!ovlrd->overwrite && ovl_type_merge_or_lower(new))
 		err = ovl_set_redirect(new, samedir);
-	else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
-		 ovl_type_merge(ovlrd.old_parent))
-		err = ovl_set_opaque_xerr(new, ovlrd.newdentry, -EXDEV);
+	else if (!ovlrd->overwrite && new_is_dir && !new_opaque &&
+		 ovl_type_merge(ovlrd->old_parent))
+		err = ovl_set_opaque_xerr(new, ovlrd->newdentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename(ofs, old_upperdir, ovlrd.olddentry,
-			    new_upperdir, ovlrd.newdentry, flags);
+	err = ovl_do_rename(ofs, old_upperdir, ovlrd->olddentry,
+			    new_upperdir, ovlrd->newdentry, flags);
+out_unlock:
 	unlock_rename(new_upperdir, old_upperdir);
 	if (err)
-		goto out_revert_creds;
+		return err;
 
-	if (ovlrd.cleanup_whiteout)
-		ovl_cleanup(ofs, old_upperdir, ovlrd.newdentry);
+	if (ovlrd->cleanup_whiteout)
+		ovl_cleanup(ofs, old_upperdir, ovlrd->newdentry);
 
-	if (ovlrd.overwrite && d_inode(new)) {
+	if (ovlrd->overwrite && d_inode(new)) {
 		if (new_is_dir)
 			clear_nlink(d_inode(new));
 		else
 			ovl_drop_nlink(new);
 	}
 
-	ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
-			 (!ovlrd.overwrite && ovl_type_origin(new)));
-	ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
+	ovl_dir_modified(ovlrd->old_parent, ovl_type_origin(old) ||
+			 (!ovlrd->overwrite && ovl_type_origin(new)));
+	ovl_dir_modified(ovlrd->new_parent, ovl_type_origin(old) ||
 			 (d_inode(new) && ovl_type_origin(new)));
 
 	/* copy ctime: */
@@ -1315,22 +1235,105 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (d_inode(new) && ovl_dentry_upper(new))
 		ovl_copyattr(d_inode(new));
 
-out_revert_creds:
+	return err;
+}
+
+static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
+		      struct dentry *old, struct inode *newdir,
+		      struct dentry *new, unsigned int flags)
+{
+	int err;
+	bool update_nlink = false;
+	bool is_dir = d_is_dir(old);
+	bool new_is_dir = d_is_dir(new);
+	const struct cred *old_cred = NULL;
+	struct ovl_renamedata ovlrd = {
+		.old_parent		= old->d_parent,
+		.old_dentry		= old,
+		.new_parent		= new->d_parent,
+		.new_dentry		= new,
+		.flags			= flags,
+		.cleanup_whiteout	= false,
+		.overwrite		= !(flags & RENAME_EXCHANGE),
+	};
+	LIST_HEAD(list);
+
+	err = -EINVAL;
+	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
+		goto out;
+
+	ovlrd.flags &= ~RENAME_NOREPLACE;
+
+	/* Don't copy up directory trees */
+	err = -EXDEV;
+	if (!ovl_can_move(old))
+		goto out;
+	if (!ovlrd.overwrite && !ovl_can_move(new))
+		goto out;
+
+	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
+		err = ovl_check_empty_dir(new, &list);
+		if (err)
+			goto out;
+	}
+
+	if (ovlrd.overwrite) {
+		if (ovl_lower_positive(old)) {
+			if (!ovl_dentry_is_whiteout(new)) {
+				/* Whiteout source */
+				ovlrd.flags |= RENAME_WHITEOUT;
+			} else {
+				/* Switch whiteouts */
+				ovlrd.flags |= RENAME_EXCHANGE;
+			}
+		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
+			ovlrd.flags |= RENAME_EXCHANGE;
+			ovlrd.cleanup_whiteout = true;
+		}
+	}
+
+	err = ovl_copy_up(old);
+	if (err)
+		goto out;
+
+	err = ovl_copy_up(new->d_parent);
+	if (err)
+		goto out;
+	if (!ovlrd.overwrite) {
+		err = ovl_copy_up(new);
+		if (err)
+			goto out;
+	} else if (d_inode(new)) {
+		err = ovl_nlink_start(new);
+		if (err)
+			goto out;
+
+		update_nlink = true;
+	}
+
+	if (!update_nlink) {
+		/* ovl_nlink_start() took ovl_want_write() */
+		err = ovl_want_write(old);
+		if (err)
+			goto out;
+	}
+
+	old_cred = ovl_override_creds(old->d_sb);
+
+	err = do_ovl_rename(&ovlrd, &list);
+
 	ovl_revert_creds(old_cred);
 	if (update_nlink)
 		ovl_nlink_end(new);
 	else
 		ovl_drop_write(old);
-out:
+
 	dput(ovlrd.newdentry);
 	dput(ovlrd.olddentry);
 	dput(ovlrd.opaquedir);
+out:
 	ovl_cache_free(&list);
 	return err;
-
-out_unlock:
-	unlock_rename(new_upperdir, old_upperdir);
-	goto out_revert_creds;
 }
 
 static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,

-- 
2.47.3

[PATCH v3 35/42] ovl: port ovl_rename() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 0812bb4ee4f6..0030f5a69d22 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1246,7 +1246,6 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	bool update_nlink = false;
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	const struct cred *old_cred = NULL;
 	struct ovl_renamedata ovlrd = {
 		.old_parent		= old->d_parent,
 		.old_dentry		= old,
@@ -1318,11 +1317,9 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 			goto out;
 	}
 
-	old_cred = ovl_override_creds(old->d_sb);
-
+	with_ovl_creds(old->d_sb)
 		err = do_ovl_rename(&ovlrd, &list);
 
-	ovl_revert_creds(old_cred);
 	if (update_nlink)
 		ovl_nlink_end(new);
 	else

-- 
2.47.3

[PATCH v3 36/42] ovl: port ovl_copyfile() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index e375c7306051..42a77876a36d 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -531,7 +531,6 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 {
 	struct inode *inode_out = file_inode(file_out);
 	struct file *realfile_in, *realfile_out;
-	const struct cred *old_cred;
 	loff_t ret;
 
 	inode_lock(inode_out);
@@ -553,25 +552,27 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 	if (IS_ERR(realfile_in))
 		goto out_unlock;
 
-	old_cred = ovl_override_creds(file_inode(file_out)->i_sb);
+	with_ovl_creds(file_inode(file_out)->i_sb) {
 		switch (op) {
 			case OVL_COPY:
 				ret = vfs_copy_file_range(realfile_in, pos_in,
-					  realfile_out, pos_out, len, flags);
+							  realfile_out, pos_out,
+							  len, flags);
 				break;
 
 			case OVL_CLONE:
 				ret = vfs_clone_file_range(realfile_in, pos_in,
-					   realfile_out, pos_out, len, flags);
+							   realfile_out,
+							   pos_out, len, flags);
 				break;
 
 			case OVL_DEDUPE:
 				ret = vfs_dedupe_file_range_one(realfile_in, pos_in,
-						realfile_out, pos_out, len,
-						flags);
+								realfile_out, pos_out,
+								len, flags);
 				break;
 		}
-	ovl_revert_creds(old_cred);
+	}
 
 	/* Update size */
 	ovl_file_modified(file_out);

-- 
2.47.3

[PATCH v3 37/42] ovl: refactor ovl_lookup()

[Christian Brauner]

Split the core into a separate helper in preparation of converting the
caller to the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 49874525cf52..4368f9f6ff9c 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1070,57 +1070,44 @@ static bool ovl_check_follow_redirect(struct ovl_lookup_data *d)
 	return true;
 }
 
-struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
-			  unsigned int flags)
+struct ovl_lookup_ctx {
+	struct dentry *dentry;
+	struct ovl_entry *oe;
+	struct ovl_path *stack;
+	struct ovl_path *origin_path;
+	struct dentry *upperdentry;
+	struct dentry *index;
+	struct inode *inode;
+	unsigned int ctr;
+};
+
+static int do_ovl_lookup(struct ovl_lookup_ctx *ctx, struct ovl_lookup_data *d)
 {
-	struct ovl_entry *oe = NULL;
-	const struct cred *old_cred;
+	struct dentry *dentry = ctx->dentry;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct ovl_entry *poe = OVL_E(dentry->d_parent);
 	struct ovl_entry *roe = OVL_E(dentry->d_sb->s_root);
-	struct ovl_path *stack = NULL, *origin_path = NULL;
-	struct dentry *upperdir, *upperdentry = NULL;
-	struct dentry *origin = NULL;
-	struct dentry *index = NULL;
-	unsigned int ctr = 0;
-	struct inode *inode = NULL;
-	bool upperopaque = false;
 	bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
+	struct dentry *upperdir;
 	struct dentry *this;
-	unsigned int i;
-	int err;
+	struct dentry *origin = NULL;
+	bool upperopaque = false;
 	bool uppermetacopy = false;
 	int metacopy_size = 0;
-	struct ovl_lookup_data d = {
-		.sb = dentry->d_sb,
-		.dentry = dentry,
-		.name = dentry->d_name,
-		.is_dir = false,
-		.opaque = false,
-		.stop = false,
-		.last = check_redirect ? false : !ovl_numlower(poe),
-		.redirect = NULL,
-		.upperredirect = NULL,
-		.metacopy = 0,
-	};
-
-	if (dentry->d_name.len > ofs->namelen)
-		return ERR_PTR(-ENAMETOOLONG);
+	unsigned int i;
+	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
 	upperdir = ovl_dentry_upper(dentry->d_parent);
 	if (upperdir) {
-		d.layer = &ofs->layers[0];
-		err = ovl_lookup_layer(upperdir, &d, &upperdentry, true);
+		d->layer = &ofs->layers[0];
+		err = ovl_lookup_layer(upperdir, d, &ctx->upperdentry, true);
 		if (err)
-			goto out;
+			return err;
 
-		if (upperdentry && upperdentry->d_flags & DCACHE_OP_REAL) {
-			dput(upperdentry);
-			err = -EREMOTE;
-			goto out;
-		}
-		if (upperdentry && !d.is_dir) {
+		if (ctx->upperdentry && ctx->upperdentry->d_flags & DCACHE_OP_REAL)
+			return -EREMOTE;
+
+		if (ctx->upperdentry && !d->is_dir) {
 			/*
 			 * Lookup copy up origin by decoding origin file handle.
 			 * We may get a disconnected dentry, which is fine,
@@ -1131,50 +1118,50 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			 * number - it's the same as if we held a reference
 			 * to a dentry in lower layer that was moved under us.
 			 */
-			err = ovl_check_origin(ofs, upperdentry, &origin_path);
+			err = ovl_check_origin(ofs, ctx->upperdentry, &ctx->origin_path);
 			if (err)
-				goto out_put_upper;
+				return err;
 
-			if (d.metacopy)
+			if (d->metacopy)
 				uppermetacopy = true;
-			metacopy_size = d.metacopy;
+			metacopy_size = d->metacopy;
 		}
 
-		if (d.redirect) {
+		if (d->redirect) {
 			err = -ENOMEM;
-			d.upperredirect = kstrdup(d.redirect, GFP_KERNEL);
-			if (!d.upperredirect)
-				goto out_put_upper;
-			if (d.redirect[0] == '/')
+			d->upperredirect = kstrdup(d->redirect, GFP_KERNEL);
+			if (!d->upperredirect)
+				return err;
+			if (d->redirect[0] == '/')
 				poe = roe;
 		}
-		upperopaque = d.opaque;
+		upperopaque = d->opaque;
 	}
 
-	if (!d.stop && ovl_numlower(poe)) {
+	if (!d->stop && ovl_numlower(poe)) {
 		err = -ENOMEM;
-		stack = ovl_stack_alloc(ofs->numlayer - 1);
-		if (!stack)
-			goto out_put_upper;
+		ctx->stack = ovl_stack_alloc(ofs->numlayer - 1);
+		if (!ctx->stack)
+			return err;
 	}
 
-	for (i = 0; !d.stop && i < ovl_numlower(poe); i++) {
+	for (i = 0; !d->stop && i < ovl_numlower(poe); i++) {
 		struct ovl_path lower = ovl_lowerstack(poe)[i];
 
-		if (!ovl_check_follow_redirect(&d)) {
+		if (!ovl_check_follow_redirect(d)) {
 			err = -EPERM;
-			goto out_put;
+			return err;
 		}
 
 		if (!check_redirect)
-			d.last = i == ovl_numlower(poe) - 1;
-		else if (d.is_dir || !ofs->numdatalayer)
-			d.last = lower.layer->idx == ovl_numlower(roe);
+			d->last = i == ovl_numlower(poe) - 1;
+		else if (d->is_dir || !ofs->numdatalayer)
+			d->last = lower.layer->idx == ovl_numlower(roe);
 
-		d.layer = lower.layer;
-		err = ovl_lookup_layer(lower.dentry, &d, &this, false);
+		d->layer = lower.layer;
+		err = ovl_lookup_layer(lower.dentry, d, &this, false);
 		if (err)
-			goto out_put;
+			return err;
 
 		if (!this)
 			continue;
@@ -1183,11 +1170,11 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		 * If no origin fh is stored in upper of a merge dir, store fh
 		 * of lower dir and set upper parent "impure".
 		 */
-		if (upperdentry && !ctr && !ofs->noxattr && d.is_dir) {
-			err = ovl_fix_origin(ofs, dentry, this, upperdentry);
+		if (ctx->upperdentry && !ctx->ctr && !ofs->noxattr && d->is_dir) {
+			err = ovl_fix_origin(ofs, dentry, this, ctx->upperdentry);
 			if (err) {
 				dput(this);
-				goto out_put;
+				return err;
 			}
 		}
 
@@ -1200,23 +1187,23 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		 * matches the dentry found using path based lookup,
 		 * otherwise error out.
 		 */
-		if (upperdentry && !ctr &&
-		    ((d.is_dir && ovl_verify_lower(dentry->d_sb)) ||
-		     (!d.is_dir && ofs->config.index && origin_path))) {
-			err = ovl_verify_origin(ofs, upperdentry, this, false);
+		if (ctx->upperdentry && !ctx->ctr &&
+		    ((d->is_dir && ovl_verify_lower(dentry->d_sb)) ||
+		     (!d->is_dir && ofs->config.index && ctx->origin_path))) {
+			err = ovl_verify_origin(ofs, ctx->upperdentry, this, false);
 			if (err) {
 				dput(this);
-				if (d.is_dir)
+				if (d->is_dir)
 					break;
-				goto out_put;
+				return err;
 			}
 			origin = this;
 		}
 
-		if (!upperdentry && !d.is_dir && !ctr && d.metacopy)
-			metacopy_size = d.metacopy;
+		if (!ctx->upperdentry && !d->is_dir && !ctx->ctr && d->metacopy)
+			metacopy_size = d->metacopy;
 
-		if (d.metacopy && ctr) {
+		if (d->metacopy && ctx->ctr) {
 			/*
 			 * Do not store intermediate metacopy dentries in
 			 * lower chain, except top most lower metacopy dentry.
@@ -1226,15 +1213,15 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			dput(this);
 			this = NULL;
 		} else {
-			stack[ctr].dentry = this;
-			stack[ctr].layer = lower.layer;
-			ctr++;
+			ctx->stack[ctx->ctr].dentry = this;
+			ctx->stack[ctx->ctr].layer = lower.layer;
+			ctx->ctr++;
 		}
 
-		if (d.stop)
+		if (d->stop)
 			break;
 
-		if (d.redirect && d.redirect[0] == '/' && poe != roe) {
+		if (d->redirect && d->redirect[0] == '/' && poe != roe) {
 			poe = roe;
 			/* Find the current layer on the root dentry */
 			i = lower.layer->idx - 1;
@@ -1245,12 +1232,12 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	 * Defer lookup of lowerdata in data-only layers to first access.
 	 * Don't require redirect=follow and metacopy=on in this case.
 	 */
-	if (d.metacopy && ctr && ofs->numdatalayer && d.absolute_redirect) {
-		d.metacopy = 0;
-		ctr++;
-	} else if (!ovl_check_follow_redirect(&d)) {
+	if (d->metacopy && ctx->ctr && ofs->numdatalayer && d->absolute_redirect) {
+		d->metacopy = 0;
+		ctx->ctr++;
+	} else if (!ovl_check_follow_redirect(d)) {
 		err = -EPERM;
-		goto out_put;
+		return err;
 	}
 
 	/*
@@ -1261,20 +1248,20 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	 * For metacopy dentry, path based lookup will find lower dentries.
 	 * Just make sure a corresponding data dentry has been found.
 	 */
-	if (d.metacopy || (uppermetacopy && !ctr)) {
+	if (d->metacopy || (uppermetacopy && !ctx->ctr)) {
 		pr_warn_ratelimited("metacopy with no lower data found - abort lookup (%pd2)\n",
 				    dentry);
 		err = -EIO;
-		goto out_put;
-	} else if (!d.is_dir && upperdentry && !ctr && origin_path) {
-		if (WARN_ON(stack != NULL)) {
+		return err;
+	} else if (!d->is_dir && ctx->upperdentry && !ctx->ctr && ctx->origin_path) {
+		if (WARN_ON(ctx->stack != NULL)) {
 			err = -EIO;
-			goto out_put;
+			return err;
 		}
-		stack = origin_path;
-		ctr = 1;
-		origin = origin_path->dentry;
-		origin_path = NULL;
+		ctx->stack = ctx->origin_path;
+		ctx->ctr = 1;
+		origin = ctx->origin_path->dentry;
+		ctx->origin_path = NULL;
 	}
 
 	/*
@@ -1296,38 +1283,39 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	 * is enabled and if upper had an ORIGIN xattr.
 	 *
 	 */
-	if (!upperdentry && ctr)
-		origin = stack[0].dentry;
+	if (!ctx->upperdentry && ctx->ctr)
+		origin = ctx->stack[0].dentry;
 
 	if (origin && ovl_indexdir(dentry->d_sb) &&
-	    (!d.is_dir || ovl_index_all(dentry->d_sb))) {
-		index = ovl_lookup_index(ofs, upperdentry, origin, true);
-		if (IS_ERR(index)) {
-			err = PTR_ERR(index);
-			index = NULL;
-			goto out_put;
+	    (!d->is_dir || ovl_index_all(dentry->d_sb))) {
+		ctx->index = ovl_lookup_index(ofs, ctx->upperdentry, origin, true);
+		if (IS_ERR(ctx->index)) {
+			err = PTR_ERR(ctx->index);
+			ctx->index = NULL;
+			return err;
 		}
 	}
 
-	if (ctr) {
-		oe = ovl_alloc_entry(ctr);
+	if (ctx->ctr) {
+		ctx->oe = ovl_alloc_entry(ctx->ctr);
 		err = -ENOMEM;
-		if (!oe)
-			goto out_put;
+		if (!ctx->oe)
+			return err;
 
-		ovl_stack_cpy(ovl_lowerstack(oe), stack, ctr);
+		ovl_stack_cpy(ovl_lowerstack(ctx->oe), ctx->stack, ctx->ctr);
 	}
 
 	if (upperopaque)
 		ovl_dentry_set_opaque(dentry);
-	if (d.xwhiteouts)
+	if (d->xwhiteouts)
 		ovl_dentry_set_xwhiteouts(dentry);
 
-	if (upperdentry)
+	if (ctx->upperdentry)
 		ovl_dentry_set_upper_alias(dentry);
-	else if (index) {
+	else if (ctx->index) {
+		char *upperredirect;
 		struct path upperpath = {
-			.dentry = upperdentry = dget(index),
+			.dentry = ctx->upperdentry = dget(ctx->index),
 			.mnt = ovl_upper_mnt(ofs),
 		};
 
@@ -1336,77 +1324,97 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 		 * assignment happens only if upperdentry is non-NULL, and
 		 * this one only if upperdentry is NULL.
 		 */
-		d.upperredirect = ovl_get_redirect_xattr(ofs, &upperpath, 0);
-		if (IS_ERR(d.upperredirect)) {
-			err = PTR_ERR(d.upperredirect);
-			d.upperredirect = NULL;
-			goto out_free_oe;
-		}
+		upperredirect = ovl_get_redirect_xattr(ofs, &upperpath, 0);
+		if (IS_ERR(upperredirect))
+			return PTR_ERR(upperredirect);
+		d->upperredirect = upperredirect;
 
 		err = ovl_check_metacopy_xattr(ofs, &upperpath, NULL);
 		if (err < 0)
-			goto out_free_oe;
-		d.metacopy = uppermetacopy = err;
+			return err;
+		d->metacopy = uppermetacopy = err;
 		metacopy_size = err;
 
-		if (!ovl_check_follow_redirect(&d)) {
+		if (!ovl_check_follow_redirect(d)) {
 			err = -EPERM;
-			goto out_free_oe;
+			return err;
 		}
 	}
 
-	if (upperdentry || ctr) {
+	if (ctx->upperdentry || ctx->ctr) {
+		struct inode *inode;
 		struct ovl_inode_params oip = {
-			.upperdentry = upperdentry,
-			.oe = oe,
-			.index = index,
-			.redirect = d.upperredirect,
+			.upperdentry = ctx->upperdentry,
+			.oe = ctx->oe,
+			.index = ctx->index,
+			.redirect = d->upperredirect,
 		};
 
 		/* Store lowerdata redirect for lazy lookup */
-		if (ctr > 1 && !d.is_dir && !stack[ctr - 1].dentry) {
-			oip.lowerdata_redirect = d.redirect;
-			d.redirect = NULL;
+		if (ctx->ctr > 1 && !d->is_dir && !ctx->stack[ctx->ctr - 1].dentry) {
+			oip.lowerdata_redirect = d->redirect;
+			d->redirect = NULL;
 		}
+
 		inode = ovl_get_inode(dentry->d_sb, &oip);
-		err = PTR_ERR(inode);
 		if (IS_ERR(inode))
-			goto out_free_oe;
-		if (upperdentry && !uppermetacopy)
-			ovl_set_flag(OVL_UPPERDATA, inode);
+			return PTR_ERR(inode);
+
+		ctx->inode = inode;
+		if (ctx->upperdentry && !uppermetacopy)
+			ovl_set_flag(OVL_UPPERDATA, ctx->inode);
 
 		if (metacopy_size > OVL_METACOPY_MIN_SIZE)
-			ovl_set_flag(OVL_HAS_DIGEST, inode);
+			ovl_set_flag(OVL_HAS_DIGEST, ctx->inode);
 	}
 
-	ovl_dentry_init_reval(dentry, upperdentry, OVL_I_E(inode));
+	ovl_dentry_init_reval(dentry, ctx->upperdentry, OVL_I_E(ctx->inode));
+
+	return 0;
+}
+
+struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
+			  unsigned int flags)
+{
+	const struct cred *old_cred;
+	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
+	struct ovl_entry *poe = OVL_E(dentry->d_parent);
+	bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
+	int err;
+	struct ovl_lookup_ctx ctx = {
+		.dentry = dentry,
+	};
+	struct ovl_lookup_data d = {
+		.sb	= dentry->d_sb,
+		.dentry = dentry,
+		.name	= dentry->d_name,
+		.last	= check_redirect ? false : !ovl_numlower(poe),
+	};
+
+	if (dentry->d_name.len > ofs->namelen)
+		return ERR_PTR(-ENAMETOOLONG);
+
+	old_cred = ovl_override_creds(dentry->d_sb);
+
+	err = do_ovl_lookup(&ctx, &d);
 
 	ovl_revert_creds(old_cred);
-	if (origin_path) {
-		dput(origin_path->dentry);
-		kfree(origin_path);
+	if (ctx.origin_path) {
+		dput(ctx.origin_path->dentry);
+		kfree(ctx.origin_path);
 	}
-	dput(index);
-	ovl_stack_free(stack, ctr);
+	dput(ctx.index);
+	ovl_stack_free(ctx.stack, ctx.ctr);
 	kfree(d.redirect);
-	return d_splice_alias(inode, dentry);
 
-out_free_oe:
-	ovl_free_entry(oe);
-out_put:
-	dput(index);
-	ovl_stack_free(stack, ctr);
-out_put_upper:
-	if (origin_path) {
-		dput(origin_path->dentry);
-		kfree(origin_path);
-	}
-	dput(upperdentry);
+	if (err) {
+		ovl_free_entry(ctx.oe);
+		dput(ctx.upperdentry);
 		kfree(d.upperredirect);
-out:
-	kfree(d.redirect);
-	ovl_revert_creds(old_cred);
 		return ERR_PTR(err);
+	}
+
+	return d_splice_alias(ctx.inode, dentry);
 }
 
 bool ovl_lower_positive(struct dentry *dentry)

-- 
2.47.3

[PATCH v3 38/42] ovl: port ovl_lookup() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 4368f9f6ff9c..9c0c539b3a37 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1376,7 +1376,6 @@ static int do_ovl_lookup(struct ovl_lookup_ctx *ctx, struct ovl_lookup_data *d)
 struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			  unsigned int flags)
 {
-	const struct cred *old_cred;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct ovl_entry *poe = OVL_E(dentry->d_parent);
 	bool check_redirect = (ovl_redirect_follow(ofs) || ofs->numdatalayer);
@@ -1394,11 +1393,9 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	if (dentry->d_name.len > ofs->namelen)
 		return ERR_PTR(-ENAMETOOLONG);
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-
+	with_ovl_creds(dentry->d_sb)
 		err = do_ovl_lookup(&ctx, &d);
 
-	ovl_revert_creds(old_cred);
 	if (ctx.origin_path) {
 		dput(ctx.origin_path->dentry);
 		kfree(ctx.origin_path);

-- 
2.47.3

[PATCH v3 39/42] ovl: port ovl_lower_positive() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 9c0c539b3a37..9364deac0459 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -1418,7 +1418,6 @@ bool ovl_lower_positive(struct dentry *dentry)
 {
 	struct ovl_entry *poe = OVL_E(dentry->d_parent);
 	const struct qstr *name = &dentry->d_name;
-	const struct cred *old_cred;
 	unsigned int i;
 	bool positive = false;
 	bool done = false;
@@ -1434,7 +1433,7 @@ bool ovl_lower_positive(struct dentry *dentry)
 	if (!ovl_dentry_upper(dentry))
 		return true;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	with_ovl_creds(dentry->d_sb) {
 		/* Positive upper -> have to look up lower to see whether it exists */
 		for (i = 0; !done && !positive && i < ovl_numlower(poe); i++) {
 			struct dentry *this;
@@ -1445,8 +1444,7 @@ bool ovl_lower_positive(struct dentry *dentry)
 			 * because lookup_one_positive_unlocked() will hash name
 			 * with parentpath base, which is on another (lower fs).
 			 */
-		this = lookup_one_positive_unlocked(
-				mnt_idmap(parentpath->layer->mnt),
+			this = lookup_one_positive_unlocked(mnt_idmap(parentpath->layer->mnt),
 							    &QSTR_LEN(name->name, name->len),
 							    parentpath->dentry);
 			if (IS_ERR(this)) {
@@ -1473,7 +1471,7 @@ bool ovl_lower_positive(struct dentry *dentry)
 				dput(this);
 			}
 		}
-	ovl_revert_creds(old_cred);
+	}
 
 	return positive;
 }

-- 
2.47.3

[PATCH v3 40/42] ovl: refactor ovl_fill_super()

[Christian Brauner]

Split the core into a separate helper in preparation of converting the
caller to the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 43ee4c7296a7..e3781fccaef8 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1369,53 +1369,35 @@ static void ovl_set_d_op(struct super_block *sb)
 	set_default_d_op(sb, &ovl_dentry_operations);
 }
 
-int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
+static int do_ovl_fill_super(struct fs_context *fc, struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
+	struct cred *creator_cred = (struct cred *)ofs->creator_cred;
 	struct ovl_fs_context *ctx = fc->fs_private;
-	const struct cred *old_cred = NULL;
-	struct dentry *root_dentry;
-	struct ovl_entry *oe;
 	struct ovl_layer *layers;
-	struct cred *cred;
+	struct ovl_entry *oe = NULL;
 	int err;
 
-	err = -EIO;
-	if (WARN_ON(fc->user_ns != current_user_ns()))
-		goto out_err;
-
-	ovl_set_d_op(sb);
-
-	err = -ENOMEM;
-	if (!ofs->creator_cred)
-		ofs->creator_cred = cred = prepare_creds();
-	else
-		cred = (struct cred *)ofs->creator_cred;
-	if (!cred)
-		goto out_err;
-
-	old_cred = ovl_override_creds(sb);
-
 	err = ovl_fs_params_verify(ctx, &ofs->config);
 	if (err)
-		goto out_err;
+		return err;
 
 	err = -EINVAL;
 	if (ctx->nr == 0) {
 		if (!(fc->sb_flags & SB_SILENT))
 			pr_err("missing 'lowerdir'\n");
-		goto out_err;
+		return err;
 	}
 
 	err = -ENOMEM;
 	layers = kcalloc(ctx->nr + 1, sizeof(struct ovl_layer), GFP_KERNEL);
 	if (!layers)
-		goto out_err;
+		return err;
 
 	ofs->config.lowerdirs = kcalloc(ctx->nr + 1, sizeof(char *), GFP_KERNEL);
 	if (!ofs->config.lowerdirs) {
 		kfree(layers);
-		goto out_err;
+		return err;
 	}
 	ofs->layers = layers;
 	/*
@@ -1448,12 +1430,12 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 		err = -EINVAL;
 		if (!ofs->config.workdir) {
 			pr_err("missing 'workdir'\n");
-			goto out_err;
+			return err;
 		}
 
 		err = ovl_get_upper(sb, ofs, &layers[0], &ctx->upper);
 		if (err)
-			goto out_err;
+			return err;
 
 		upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
 		if (!ovl_should_sync(ofs)) {
@@ -1461,13 +1443,13 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 			if (errseq_check(&upper_sb->s_wb_err, ofs->errseq)) {
 				err = -EIO;
 				pr_err("Cannot mount volatile when upperdir has an unseen error. Sync upperdir fs to clear state.\n");
-				goto out_err;
+				return err;
 			}
 		}
 
 		err = ovl_get_workdir(sb, ofs, &ctx->upper, &ctx->work);
 		if (err)
-			goto out_err;
+			return err;
 
 		if (!ofs->workdir)
 			sb->s_flags |= SB_RDONLY;
@@ -1478,7 +1460,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	oe = ovl_get_lowerstack(sb, ctx, ofs, layers);
 	err = PTR_ERR(oe);
 	if (IS_ERR(oe))
-		goto out_err;
+		return err;
 
 	/* If the upper fs is nonexistent, we mark overlayfs r/o too */
 	if (!ovl_upper_mnt(ofs))
@@ -1531,7 +1513,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 		sb->s_export_op = &ovl_export_fid_operations;
 
 	/* Never override disk quota limits or use reserved space */
-	cap_lower(cred->cap_effective, CAP_SYS_RESOURCE);
+	cap_lower(creator_cred->cap_effective, CAP_SYS_RESOURCE);
 
 	sb->s_magic = OVERLAYFS_SUPER_MAGIC;
 	sb->s_xattr = ovl_xattr_handlers(ofs);
@@ -1549,27 +1531,50 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_iflags |= SB_I_EVM_HMAC_UNSUPPORTED;
 
 	err = -ENOMEM;
-	root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
-	if (!root_dentry)
+	sb->s_root = ovl_get_root(sb, ctx->upper.dentry, oe);
+	if (!sb->s_root)
 		goto out_free_oe;
 
-	sb->s_root = root_dentry;
-
-	ovl_revert_creds(old_cred);
 	return 0;
 
 out_free_oe:
 	ovl_free_entry(oe);
-out_err:
-	/*
-	 * Revert creds before calling ovl_free_fs() which will call
-	 * put_cred() and put_cred() requires that the cred's that are
-	 * put are not the caller's creds, i.e., current->cred.
-	 */
-	if (old_cred)
+	return err;
+}
+
+int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
+{
+	struct ovl_fs *ofs = sb->s_fs_info;
+	const struct cred *old_cred = NULL;
+	struct cred *cred;
+	int err;
+
+	err = -EIO;
+	if (WARN_ON(fc->user_ns != current_user_ns()))
+		goto out_err;
+
+	ovl_set_d_op(sb);
+
+	err = -ENOMEM;
+	if (!ofs->creator_cred)
+		ofs->creator_cred = cred = prepare_creds();
+	else
+		cred = (struct cred *)ofs->creator_cred;
+	if (!cred)
+		goto out_err;
+
+	old_cred = ovl_override_creds(sb);
+
+	err = do_ovl_fill_super(fc, sb);
+
 	ovl_revert_creds(old_cred);
+
+out_err:
+	if (err) {
 		ovl_free_fs(ofs);
 		sb->s_fs_info = NULL;
+	}
+
 	return err;
 }
 

-- 
2.47.3

[PATCH v3 41/42] ovl: port ovl_fill_super() to cred guard

[Christian Brauner]

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e3781fccaef8..3b9b9b569e5c 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1545,8 +1545,6 @@ static int do_ovl_fill_super(struct fs_context *fc, struct super_block *sb)
 int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
-	const struct cred *old_cred = NULL;
-	struct cred *cred;
 	int err;
 
 	err = -EIO;
@@ -1555,20 +1553,16 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	ovl_set_d_op(sb);
 
+	if (!ofs->creator_cred) {
 		err = -ENOMEM;
+		ofs->creator_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;
+	}
 
-	old_cred = ovl_override_creds(sb);
-
+	with_ovl_creds(sb)
 		err = do_ovl_fill_super(fc, sb);
 
-	ovl_revert_creds(old_cred);
-
 out_err:
 	if (err) {
 		ovl_free_fs(ofs);

-- 
2.47.3

[PATCH v3 42/42] ovl: remove ovl_revert_creds()

[Christian Brauner]

The wrapper isn't needed anymore. Overlayfs completely relies on its
cleanup guard.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/overlayfs/overlayfs.h | 1 -
 fs/overlayfs/util.c      | 5 -----
 2 files changed, 6 deletions(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index eeace590ba57..41a3c0e9595b 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -437,7 +437,6 @@ int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
 const struct cred *ovl_override_creds(struct super_block *sb);
-void ovl_revert_creds(const struct cred *old_cred);
 
 EXTEND_CLASS(override_creds, _ovl, ovl_override_creds(sb), struct super_block *sb)
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index e2f2e0d17f0b..dc521f53d7a3 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -69,11 +69,6 @@ const struct cred *ovl_override_creds(struct super_block *sb)
 	return override_creds(ofs->creator_cred);
 }
 
-void ovl_revert_creds(const struct cred *old_cred)
-{
-	revert_creds(old_cred);
-}
-
 /*
  * Check if underlying fs supports file handles and try to determine encoding
  * type, in order to deduce maximum inode number used by fs.

-- 
2.47.3

Re: [PATCH v3 03/42] ovl: port ovl_create_or_link() to cred guard

[Miklos Szeredi]

On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:

> @@ -641,23 +640,17 @@ 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.
>                          */
> -               new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode,
> -                                                    old_cred);
> -               err = PTR_ERR(new_cred);
> -               if (IS_ERR(new_cred)) {
> -                       new_cred = NULL;
> -                       goto out_revert_creds;
> -               }
> +                       new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred);
> +                       if (IS_ERR(new_cred))
> +                               return PTR_ERR(new_cred);

put_cred() doesn't handle IS_ERR() pointers, AFAICS.

Re: [PATCH v3 06/42] ovl: port ovl_create_tmpfile() to cred guard

[Miklos Szeredi]

On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:

> @@ -1332,27 +1332,25 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
>         int flags = file->f_flags | OVL_OPEN_FLAGS;
>         int err;
>
> -       old_cred = ovl_override_creds(dentry->d_sb);
> +       scoped_class(override_creds_ovl, old_cred, dentry->d_sb) {
>                 new_cred = ovl_setup_cred_for_create(dentry, inode, mode, old_cred);
> -       err = PTR_ERR(new_cred);
> -       if (IS_ERR(new_cred)) {
> -               new_cred = NULL;
> -               goto out_revert_creds;
> -       }
> +               if (IS_ERR(new_cred))
> +                       return PTR_ERR(new_cred);

Same thing here.

>
>                 ovl_path_upper(dentry->d_parent, &realparentpath);
> -       realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
> -                                       mode, current_cred());
> +               realfile = backing_tmpfile_open(&file->f_path, flags,
> +                                               &realparentpath, mode,
> +                                               current_cred());

Where do we stand wrt "chars per line" thing?   checkpatch now allows
100(?) so shouldn't we take advantage of that?

Re: [PATCH v3 24/42] ovl: don't override credentials for ovl_check_whiteouts()

[Miklos Szeredi]

On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
>
> The function is only called when rdd->dentry is non-NULL:
>
> if (!err && rdd->first_maybe_whiteout && rdd->dentry)
>     err = ovl_check_whiteouts(realpath, rdd);
>
> | Caller                        | Sets rdd->dentry? | Can call ovl_check_whiteouts()? |
> |-------------------------------|-------------------|---------------------------------|
> | ovl_dir_read_merged()         | ✓ Yes (line 430)  | ✓ YES                           |
> | ovl_dir_read_impure()         | ✗ No              | ✗ NO                            |
> | ovl_check_d_type_supported()  | ✗ No              | ✗ NO                            |
> | ovl_workdir_cleanup_recurse() | ✗ No              | ✗ NO                            |
> | ovl_indexdir_cleanup()        | ✗ No              | ✗ NO                            |
>
> VFS layer (.iterate_shared file operation)
>   → ovl_iterate()
>       [CRED OVERRIDE]
>       → ovl_cache_get()
>           → ovl_dir_read_merged()
>               → ovl_dir_read()
>                   → ovl_check_whiteouts()
>       [CRED REVERT]
>
> ovl_unlink()
>   → ovl_do_remove()
>       → ovl_check_empty_dir()
>           [CRED OVERRIDE]
>           → ovl_dir_read_merged()
>               → ovl_dir_read()
>                   → ovl_check_whiteouts()
>           [CRED REVERT]
>
> ovl_rename()
>   → ovl_check_empty_dir()
>       [CRED OVERRIDE]
>       → ovl_dir_read_merged()
>           → ovl_dir_read()
>               → ovl_check_whiteouts()
>       [CRED REVERT]
>
> All valid callchains already override credentials so drop the override.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/readdir.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 1e9792cc557b..12f0bb1480d7 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -348,11 +348,7 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
>
>  static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data *rdd)
>  {
> -       int err = 0;
>         struct dentry *dentry, *dir = path->dentry;
> -       const struct cred *old_cred;
> -
> -       old_cred = ovl_override_creds(rdd->dentry->d_sb);

Myabe ovl_assert_override_creds()?

Re: [PATCH v3 06/42] ovl: port ovl_create_tmpfile() to cred guard

[Amir Goldstein]

On Fri, Nov 14, 2025 at 8:18 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
>
> > @@ -1332,27 +1332,25 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
> >         int flags = file->f_flags | OVL_OPEN_FLAGS;
> >         int err;
> >
> > -       old_cred = ovl_override_creds(dentry->d_sb);
> > +       scoped_class(override_creds_ovl, old_cred, dentry->d_sb) {
> >                 new_cred = ovl_setup_cred_for_create(dentry, inode, mode, old_cred);
> > -       err = PTR_ERR(new_cred);
> > -       if (IS_ERR(new_cred)) {
> > -               new_cred = NULL;
> > -               goto out_revert_creds;
> > -       }
> > +               if (IS_ERR(new_cred))
> > +                       return PTR_ERR(new_cred);
>
> Same thing here.
>
> >
> >                 ovl_path_upper(dentry->d_parent, &realparentpath);
> > -       realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
> > -                                       mode, current_cred());
> > +               realfile = backing_tmpfile_open(&file->f_path, flags,
> > +                                               &realparentpath, mode,
> > +                                               current_cred());
>
> Where do we stand wrt "chars per line" thing?   checkpatch now allows
> 100(?) so shouldn't we take advantage of that?

For the record, where I stand is
I don't like to see code with mixed 80 and 100 lines
unless debug msg or something,
so I wouldn't make it into one long line,
but otoh I also don't keep to strict 80 anymore,
so I won't break lines like this just for old times sake

and while at it, why are we using current_cred() and not new_cred
for clarity?

realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
                                                   mode, new_cred);


Thanks,
Amir.

Re: [PATCH v3 25/42] ovl: refactor ovl_iterate() and port to cred guard

[Miklos Szeredi]

On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:

> +       /*
> +        * With xino, we need to adjust d_ino of lower entries.
> +        * On same fs, if parent is merge, then need to adjust d_ino for '..',
> +        * and if dir is impure then need to adjust d_ino for copied up entries.
> +        * Otherwise, we can iterate the real dir directly.
> +        */
> +       if (!ovl_xino_bits(ofs) &&
> +           !(ovl_same_fs(ofs) &&
> +             (ovl_is_impure_dir(file) ||
> +              OVL_TYPE_MERGE(ovl_path_type(dir->d_parent)))))
> +               return iterate_dir(od->realfile, ctx);

If this condition was confusing before, it's even more confusing now.
 What about

static bool ovl_need_adjust_d_ino(struct file *file)
{
        struct dentry *dentry = file->f_path.dentry;
        struct ovl_fs *ofs = OVL_FS(dentry->d_sb);

        /* If parent is merge, then need to adjust d_ino for '..' */
        if (ovl_xino_bits(ofs))
                return true;

        /* Can't do consistent inode numbering */
        if (!ovl_same_fs(ofs))
                return false;

        /* If dir is impure then need to adjust d_ino for copied up entries */
        if (ovl_is_impure_dir(file) ||
OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent)))
                return true;

        /* Pure: no need to adjust d_ino */
        return false;
}

>
> +static int ovl_iterate(struct file *file, struct dir_context *ctx)
> +{
> +       struct ovl_dir_file *od = file->private_data;
> +
> +       if (!ctx->pos)
> +               ovl_dir_reset(file);
> +
> +       with_ovl_creds(file_dentry(file)->d_sb) {
> +               if (od->is_real)
> +                       return ovl_iterate_real(file, ctx);

        if (od->is_real) {
                if (ovl_need_d_ino_adjust(file))
                        return ovl_iterate_real(file, ctx);
                else
                        return iterate_dir(od->realfile, ctx);
        }

Re: [PATCH v3 06/42] ovl: port ovl_create_tmpfile() to cred guard

[Miklos Szeredi]

On Fri, 14 Nov 2025 at 09:30, Amir Goldstein <amir73il@gmail.com> wrote:

> For the record, where I stand is
> I don't like to see code with mixed 80 and 100 lines
> unless debug msg or something,
> so I wouldn't make it into one long line,
> but otoh I also don't keep to strict 80 anymore,
> so I won't break lines like this just for old times sake

Here the advantage of not adding more splits (and not removing them
either) would be less churn -> easier review.  But it's not a big
deal.

> and while at it, why are we using current_cred() and not new_cred
> for clarity?
>
> realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
>                                                    mode, new_cred);

I think both are okay.

Thanks,
Miklos

Re: [PATCH v3 03/42] ovl: port ovl_create_or_link() to cred guard

[Christian Brauner]

On Fri, Nov 14, 2025 at 08:09:35AM +0100, Miklos Szeredi wrote:
> On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
> 
> > @@ -641,23 +640,17 @@ 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.
> >                          */
> > -               new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode,
> > -                                                    old_cred);
> > -               err = PTR_ERR(new_cred);
> > -               if (IS_ERR(new_cred)) {
> > -                       new_cred = NULL;
> > -                       goto out_revert_creds;
> > -               }
> > +                       new_cred = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred);
> > +                       if (IS_ERR(new_cred))
> > +                               return PTR_ERR(new_cred);
> 
> put_cred() doesn't handle IS_ERR() pointers, AFAICS.

The function doesn't but the cleanup macro (as is customary) does:
DEFINE_FREE(put_cred, struct cred *, if (!IS_ERR_OR_NULL(_T)) put_cred(_T))

Re: [PATCH v3 24/42] ovl: don't override credentials for ovl_check_whiteouts()

[Christian Brauner]

On Fri, Nov 14, 2025 at 09:11:50AM +0100, Miklos Szeredi wrote:
> On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
> >
> > The function is only called when rdd->dentry is non-NULL:
> >
> > if (!err && rdd->first_maybe_whiteout && rdd->dentry)
> >     err = ovl_check_whiteouts(realpath, rdd);
> >
> > | Caller                        | Sets rdd->dentry? | Can call ovl_check_whiteouts()? |
> > |-------------------------------|-------------------|---------------------------------|
> > | ovl_dir_read_merged()         | ✓ Yes (line 430)  | ✓ YES                           |
> > | ovl_dir_read_impure()         | ✗ No              | ✗ NO                            |
> > | ovl_check_d_type_supported()  | ✗ No              | ✗ NO                            |
> > | ovl_workdir_cleanup_recurse() | ✗ No              | ✗ NO                            |
> > | ovl_indexdir_cleanup()        | ✗ No              | ✗ NO                            |
> >
> > VFS layer (.iterate_shared file operation)
> >   → ovl_iterate()
> >       [CRED OVERRIDE]
> >       → ovl_cache_get()
> >           → ovl_dir_read_merged()
> >               → ovl_dir_read()
> >                   → ovl_check_whiteouts()
> >       [CRED REVERT]
> >
> > ovl_unlink()
> >   → ovl_do_remove()
> >       → ovl_check_empty_dir()
> >           [CRED OVERRIDE]
> >           → ovl_dir_read_merged()
> >               → ovl_dir_read()
> >                   → ovl_check_whiteouts()
> >           [CRED REVERT]
> >
> > ovl_rename()
> >   → ovl_check_empty_dir()
> >       [CRED OVERRIDE]
> >       → ovl_dir_read_merged()
> >           → ovl_dir_read()
> >               → ovl_check_whiteouts()
> >       [CRED REVERT]
> >
> > All valid callchains already override credentials so drop the override.
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/overlayfs/readdir.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> > index 1e9792cc557b..12f0bb1480d7 100644
> > --- a/fs/overlayfs/readdir.c
> > +++ b/fs/overlayfs/readdir.c
> > @@ -348,11 +348,7 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
> >
> >  static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data *rdd)
> >  {
> > -       int err = 0;
> >         struct dentry *dentry, *dir = path->dentry;
> > -       const struct cred *old_cred;
> > -
> > -       old_cred = ovl_override_creds(rdd->dentry->d_sb);
> 
> Myabe ovl_assert_override_creds()?

Yeah, I'm thinking about a follow-up series to this one to add a few
asserts in there.

Re: [PATCH v3 03/42] ovl: port ovl_create_or_link() to cred guard

[Miklos Szeredi]

On Fri, 14 Nov 2025 at 09:53, Christian Brauner <brauner@kernel.org> wrote:

> The function doesn't but the cleanup macro (as is customary) does:
> DEFINE_FREE(put_cred, struct cred *, if (!IS_ERR_OR_NULL(_T)) put_cred(_T))

Ah, missed that.

Thanks,
Miklos

Re: [PATCH v3 25/42] ovl: refactor ovl_iterate() and port to cred guard

[Amir Goldstein]

On Fri, Nov 14, 2025 at 9:40 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
>
> > +       /*
> > +        * With xino, we need to adjust d_ino of lower entries.
> > +        * On same fs, if parent is merge, then need to adjust d_ino for '..',
> > +        * and if dir is impure then need to adjust d_ino for copied up entries.
> > +        * Otherwise, we can iterate the real dir directly.
> > +        */
> > +       if (!ovl_xino_bits(ofs) &&
> > +           !(ovl_same_fs(ofs) &&
> > +             (ovl_is_impure_dir(file) ||
> > +              OVL_TYPE_MERGE(ovl_path_type(dir->d_parent)))))
> > +               return iterate_dir(od->realfile, ctx);
>
> If this condition was confusing before, it's even more confusing now.

Indeed.

>  What about
>
> static bool ovl_need_adjust_d_ino(struct file *file)
> {
>         struct dentry *dentry = file->f_path.dentry;
>         struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
>
>         /* If parent is merge, then need to adjust d_ino for '..' */
>         if (ovl_xino_bits(ofs))
>                 return true;
>
>         /* Can't do consistent inode numbering */
>         if (!ovl_same_fs(ofs))
>                 return false;
>
>         /* If dir is impure then need to adjust d_ino for copied up entries */
>         if (ovl_is_impure_dir(file) ||
> OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent)))
>                 return true;
>
>         /* Pure: no need to adjust d_ino */
>         return false;
> }
>

I like it.

> >
> > +static int ovl_iterate(struct file *file, struct dir_context *ctx)
> > +{
> > +       struct ovl_dir_file *od = file->private_data;
> > +
> > +       if (!ctx->pos)
> > +               ovl_dir_reset(file);
> > +
> > +       with_ovl_creds(file_dentry(file)->d_sb) {
> > +               if (od->is_real)
> > +                       return ovl_iterate_real(file, ctx);
>
>         if (od->is_real) {
>                 if (ovl_need_d_ino_adjust(file))
>                         return ovl_iterate_real(file, ctx);
>                 else
>                         return iterate_dir(od->realfile, ctx);
>         }

I find it very natural code flow that

if (ovl_need_d_ino_adjust(file))
         return ovl_iterate_real(file, ctx);

is inside ovl_iterate_real()
because it is literally the case of iterating real.

but if you insist that it stays out, I'd prefer:

with_ovl_creds(file_dentry(file)->d_sb) {
    if (!od->is_real)
         ovl_iterate_merged(file, ctx);
    else if (ovl_need_d_ino_adjust(file))
         return ovl_iterate_real(file, ctx);
    else
         return iterate_dir(od->realfile, ctx);
}

Thanks,
Amir.

Re: [PATCH v3 25/42] ovl: refactor ovl_iterate() and port to cred guard

[Christian Brauner]

On Fri, Nov 14, 2025 at 09:40:00AM +0100, Miklos Szeredi wrote:
> On Thu, 13 Nov 2025 at 22:32, Christian Brauner <brauner@kernel.org> wrote:
> 
> > +       /*
> > +        * With xino, we need to adjust d_ino of lower entries.
> > +        * On same fs, if parent is merge, then need to adjust d_ino for '..',
> > +        * and if dir is impure then need to adjust d_ino for copied up entries.
> > +        * Otherwise, we can iterate the real dir directly.
> > +        */
> > +       if (!ovl_xino_bits(ofs) &&
> > +           !(ovl_same_fs(ofs) &&
> > +             (ovl_is_impure_dir(file) ||
> > +              OVL_TYPE_MERGE(ovl_path_type(dir->d_parent)))))
> > +               return iterate_dir(od->realfile, ctx);
> 
> If this condition was confusing before, it's even more confusing now.
>  What about

folded

> 
> static bool ovl_need_adjust_d_ino(struct file *file)
> {
>         struct dentry *dentry = file->f_path.dentry;
>         struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> 
>         /* If parent is merge, then need to adjust d_ino for '..' */
>         if (ovl_xino_bits(ofs))
>                 return true;
> 
>         /* Can't do consistent inode numbering */
>         if (!ovl_same_fs(ofs))
>                 return false;
> 
>         /* If dir is impure then need to adjust d_ino for copied up entries */
>         if (ovl_is_impure_dir(file) ||
> OVL_TYPE_MERGE(ovl_path_type(dentry->d_parent)))
>                 return true;
> 
>         /* Pure: no need to adjust d_ino */
>         return false;
> }
> 
> >
> > +static int ovl_iterate(struct file *file, struct dir_context *ctx)
> > +{
> > +       struct ovl_dir_file *od = file->private_data;
> > +
> > +       if (!ctx->pos)
> > +               ovl_dir_reset(file);
> > +
> > +       with_ovl_creds(file_dentry(file)->d_sb) {
> > +               if (od->is_real)
> > +                       return ovl_iterate_real(file, ctx);
> 
>         if (od->is_real) {
>                 if (ovl_need_d_ino_adjust(file))
>                         return ovl_iterate_real(file, ctx);
>                 else
>                         return iterate_dir(od->realfile, ctx);
>         }

All sounds good. Done.

Re: [PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Amir Goldstein]

On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Extract the code that runs under overridden credentials into a separate
> do_ovl_rename() helper function. Error handling is simplified. The
> helper returns errors directly instead of using goto labels.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---

For the record, the only way I could review this patch is by manually
moving the helper and doing diff, so while I approve the code
I think this is unreviewable as it is posted.

>  fs/overlayfs/dir.c | 277 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 140 insertions(+), 137 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 052929b9b99d..0812bb4ee4f6 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1099,107 +1099,28 @@ struct ovl_renamedata {
>         bool overwrite;
>  };
>
> -static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
> -                     struct dentry *old, struct inode *newdir,
> -                     struct dentry *new, unsigned int flags)
> +static int do_ovl_rename(struct ovl_renamedata *ovlrd, struct list_head *list)
>  {
> -       int err;
> -       struct dentry *old_upperdir;
> -       struct dentry *new_upperdir;
> -       struct dentry *trap, *de;
> -       bool old_opaque;
> -       bool new_opaque;
> -       bool update_nlink = false;
> +       struct dentry *old = ovlrd->old_dentry;
> +       struct dentry *new = ovlrd->new_dentry;
> +       struct ovl_fs *ofs = OVL_FS(old->d_sb);
> +       unsigned int flags = ovlrd->flags;
> +       struct dentry *old_upperdir = ovl_dentry_upper(ovlrd->old_parent);
> +       struct dentry *new_upperdir = ovl_dentry_upper(ovlrd->new_parent);
> +       bool samedir = ovlrd->old_parent == ovlrd->new_parent;
>         bool is_dir = d_is_dir(old);
>         bool new_is_dir = d_is_dir(new);
> -       const struct cred *old_cred = NULL;
> -       struct ovl_fs *ofs = OVL_FS(old->d_sb);
> -       struct ovl_renamedata ovlrd = {
> -               .old_parent             = old->d_parent,
> -               .old_dentry             = old,
> -               .new_parent             = new->d_parent,
> -               .new_dentry             = new,
> -               .flags                  = flags,
> -               .cleanup_whiteout       = false,
> -               .overwrite              = !(flags & RENAME_EXCHANGE),
> -       };
> -       LIST_HEAD(list);
> -       bool samedir = ovlrd.old_parent == ovlrd.new_parent;
> -
> -       err = -EINVAL;
> -       if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
> -               goto out;
> -
> -       ovlrd.flags &= ~RENAME_NOREPLACE;
> -
> -       /* Don't copy up directory trees */
> -       err = -EXDEV;
> -       if (!ovl_can_move(old))
> -               goto out;
> -       if (!ovlrd.overwrite && !ovl_can_move(new))
> -               goto out;
> -
> -       if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
> -               err = ovl_check_empty_dir(new, &list);
> -               if (err)
> -                       goto out;
> -       }
> -
> -       if (ovlrd.overwrite) {
> -               if (ovl_lower_positive(old)) {
> -                       if (!ovl_dentry_is_whiteout(new)) {
> -                               /* Whiteout source */
> -                               ovlrd.flags |= RENAME_WHITEOUT;
> -                       } else {
> -                               /* Switch whiteouts */
> -                               ovlrd.flags |= RENAME_EXCHANGE;
> -                       }
> -               } else if (is_dir && ovl_dentry_is_whiteout(new)) {
> -                       ovlrd.flags |= RENAME_EXCHANGE;
> -                       ovlrd.cleanup_whiteout = true;
> -               }
> -       }
> -
> -       err = ovl_copy_up(old);
> -       if (err)
> -               goto out;
> -
> -       err = ovl_copy_up(ovlrd.new_parent);
> -       if (err)
> -               goto out;
> -       if (!ovlrd.overwrite) {
> -               err = ovl_copy_up(new);
> -               if (err)
> -                       goto out;
> -       } else if (d_inode(new)) {
> -               err = ovl_nlink_start(new);
> -               if (err)
> -                       goto out;
> -
> -               update_nlink = true;
> -       }
> -
> -       if (!update_nlink) {
> -               /* ovl_nlink_start() took ovl_want_write() */
> -               err = ovl_want_write(old);
> -               if (err)
> -                       goto out;
> -       }
> -
> -       old_cred = ovl_override_creds(old->d_sb);
> +       struct dentry *trap, *de;
> +       bool old_opaque, new_opaque;
> +       int err;
>
> -       if (!list_empty(&list)) {
> -               ovlrd.opaquedir = ovl_clear_empty(new, &list);
> -               err = PTR_ERR(ovlrd.opaquedir);
> -               if (IS_ERR(ovlrd.opaquedir)) {
> -                       ovlrd.opaquedir = NULL;
> -                       goto out_revert_creds;
> -               }
> +       if (!list_empty(list)) {
> +               de = ovl_clear_empty(new, list);
> +               if (IS_ERR(de))
> +                       return PTR_ERR(de);
> +               ovlrd->opaquedir = de;
>         }
>
> -       old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
> -       new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
> -
>         if (!samedir) {
>                 /*
>                  * When moving a merge dir or non-dir with copy up origin into
> @@ -1208,32 +1129,30 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>                  * lookup the origin inodes of the entries to fill d_ino.
>                  */
>                 if (ovl_type_origin(old)) {
> -                       err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
> +                       err = ovl_set_impure(ovlrd->new_parent, new_upperdir);
>                         if (err)
> -                               goto out_revert_creds;
> +                               return err;
>                 }
> -               if (!ovlrd.overwrite && ovl_type_origin(new)) {
> -                       err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
> +               if (!ovlrd->overwrite && ovl_type_origin(new)) {
> +                       err = ovl_set_impure(ovlrd->old_parent, old_upperdir);
>                         if (err)
> -                               goto out_revert_creds;
> +                               return err;
>                 }
>         }
>
>         trap = lock_rename(new_upperdir, old_upperdir);
> -       if (IS_ERR(trap)) {
> -               err = PTR_ERR(trap);
> -               goto out_revert_creds;
> -       }
> +       if (IS_ERR(trap))
> +               return PTR_ERR(trap);
>
>         de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir,
>                               old->d_name.len);
>         err = PTR_ERR(de);
>         if (IS_ERR(de))
>                 goto out_unlock;
> -       ovlrd.olddentry = de;
> +       ovlrd->olddentry = de;
>
>         err = -ESTALE;
> -       if (!ovl_matches_upper(old, ovlrd.olddentry))
> +       if (!ovl_matches_upper(old, ovlrd->olddentry))
>                 goto out_unlock;
>
>         de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
> @@ -1241,73 +1160,74 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>         err = PTR_ERR(de);
>         if (IS_ERR(de))
>                 goto out_unlock;
> -       ovlrd.newdentry = de;
> +       ovlrd->newdentry = de;
>
>         old_opaque = ovl_dentry_is_opaque(old);
>         new_opaque = ovl_dentry_is_opaque(new);
>
>         err = -ESTALE;
>         if (d_inode(new) && ovl_dentry_upper(new)) {
> -               if (ovlrd.opaquedir) {
> -                       if (ovlrd.newdentry != ovlrd.opaquedir)
> +               if (ovlrd->opaquedir) {
> +                       if (ovlrd->newdentry != ovlrd->opaquedir)
>                                 goto out_unlock;
>                 } else {
> -                       if (!ovl_matches_upper(new, ovlrd.newdentry))
> +                       if (!ovl_matches_upper(new, ovlrd->newdentry))
>                                 goto out_unlock;
>                 }
>         } else {
> -               if (!d_is_negative(ovlrd.newdentry)) {
> -                       if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.newdentry))
> +               if (!d_is_negative(ovlrd->newdentry)) {
> +                       if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd->newdentry))
>                                 goto out_unlock;
>                 } else {
> -                       if (ovlrd.flags & RENAME_EXCHANGE)
> +                       if (flags & RENAME_EXCHANGE)
>                                 goto out_unlock;
>                 }
>         }
>
> -       if (ovlrd.olddentry == trap)
> +       if (ovlrd->olddentry == trap)
>                 goto out_unlock;
> -       if (ovlrd.newdentry == trap)
> +       if (ovlrd->newdentry == trap)
>                 goto out_unlock;
>
> -       if (ovlrd.olddentry->d_inode == ovlrd.newdentry->d_inode)
> +       if (ovlrd->olddentry->d_inode == ovlrd->newdentry->d_inode)
>                 goto out_unlock;
>
>         err = 0;
>         if (ovl_type_merge_or_lower(old))
>                 err = ovl_set_redirect(old, samedir);
> -       else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
> -               err = ovl_set_opaque_xerr(old, ovlrd.olddentry, -EXDEV);
> +       else if (is_dir && !old_opaque && ovl_type_merge(ovlrd->new_parent))
> +               err = ovl_set_opaque_xerr(old, ovlrd->olddentry, -EXDEV);
>         if (err)
>                 goto out_unlock;
>
> -       if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
> +       if (!ovlrd->overwrite && ovl_type_merge_or_lower(new))
>                 err = ovl_set_redirect(new, samedir);
> -       else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
> -                ovl_type_merge(ovlrd.old_parent))
> -               err = ovl_set_opaque_xerr(new, ovlrd.newdentry, -EXDEV);
> +       else if (!ovlrd->overwrite && new_is_dir && !new_opaque &&
> +                ovl_type_merge(ovlrd->old_parent))
> +               err = ovl_set_opaque_xerr(new, ovlrd->newdentry, -EXDEV);
>         if (err)
>                 goto out_unlock;
>
> -       err = ovl_do_rename(ofs, old_upperdir, ovlrd.olddentry,
> -                           new_upperdir, ovlrd.newdentry, flags);
> +       err = ovl_do_rename(ofs, old_upperdir, ovlrd->olddentry,
> +                           new_upperdir, ovlrd->newdentry, flags);
> +out_unlock:
>         unlock_rename(new_upperdir, old_upperdir);
>         if (err)
> -               goto out_revert_creds;
> +               return err;
>
> -       if (ovlrd.cleanup_whiteout)
> -               ovl_cleanup(ofs, old_upperdir, ovlrd.newdentry);
> +       if (ovlrd->cleanup_whiteout)
> +               ovl_cleanup(ofs, old_upperdir, ovlrd->newdentry);
>
> -       if (ovlrd.overwrite && d_inode(new)) {
> +       if (ovlrd->overwrite && d_inode(new)) {
>                 if (new_is_dir)
>                         clear_nlink(d_inode(new));
>                 else
>                         ovl_drop_nlink(new);
>         }
>
> -       ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
> -                        (!ovlrd.overwrite && ovl_type_origin(new)));
> -       ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
> +       ovl_dir_modified(ovlrd->old_parent, ovl_type_origin(old) ||
> +                        (!ovlrd->overwrite && ovl_type_origin(new)));
> +       ovl_dir_modified(ovlrd->new_parent, ovl_type_origin(old) ||
>                          (d_inode(new) && ovl_type_origin(new)));
>
>         /* copy ctime: */
> @@ -1315,22 +1235,105 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>         if (d_inode(new) && ovl_dentry_upper(new))
>                 ovl_copyattr(d_inode(new));
>
> -out_revert_creds:
> +       return err;
> +}
> +
> +static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
> +                     struct dentry *old, struct inode *newdir,
> +                     struct dentry *new, unsigned int flags)
> +{
> +       int err;
> +       bool update_nlink = false;
> +       bool is_dir = d_is_dir(old);
> +       bool new_is_dir = d_is_dir(new);
> +       const struct cred *old_cred = NULL;
> +       struct ovl_renamedata ovlrd = {
> +               .old_parent             = old->d_parent,
> +               .old_dentry             = old,
> +               .new_parent             = new->d_parent,
> +               .new_dentry             = new,
> +               .flags                  = flags,
> +               .cleanup_whiteout       = false,
> +               .overwrite              = !(flags & RENAME_EXCHANGE),
> +       };
> +       LIST_HEAD(list);
> +
> +       err = -EINVAL;
> +       if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
> +               goto out;
> +
> +       ovlrd.flags &= ~RENAME_NOREPLACE;
> +
> +       /* Don't copy up directory trees */
> +       err = -EXDEV;
> +       if (!ovl_can_move(old))
> +               goto out;
> +       if (!ovlrd.overwrite && !ovl_can_move(new))
> +               goto out;
> +
> +       if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
> +               err = ovl_check_empty_dir(new, &list);
> +               if (err)
> +                       goto out;
> +       }
> +
> +       if (ovlrd.overwrite) {
> +               if (ovl_lower_positive(old)) {
> +                       if (!ovl_dentry_is_whiteout(new)) {
> +                               /* Whiteout source */
> +                               ovlrd.flags |= RENAME_WHITEOUT;
> +                       } else {
> +                               /* Switch whiteouts */
> +                               ovlrd.flags |= RENAME_EXCHANGE;
> +                       }
> +               } else if (is_dir && ovl_dentry_is_whiteout(new)) {
> +                       ovlrd.flags |= RENAME_EXCHANGE;
> +                       ovlrd.cleanup_whiteout = true;
> +               }
> +       }
> +
> +       err = ovl_copy_up(old);
> +       if (err)
> +               goto out;
> +
> +       err = ovl_copy_up(new->d_parent);
> +       if (err)
> +               goto out;
> +       if (!ovlrd.overwrite) {
> +               err = ovl_copy_up(new);
> +               if (err)
> +                       goto out;
> +       } else if (d_inode(new)) {
> +               err = ovl_nlink_start(new);
> +               if (err)
> +                       goto out;
> +
> +               update_nlink = true;
> +       }
> +
> +       if (!update_nlink) {
> +               /* ovl_nlink_start() took ovl_want_write() */
> +               err = ovl_want_write(old);
> +               if (err)
> +                       goto out;
> +       }
> +
> +       old_cred = ovl_override_creds(old->d_sb);
> +
> +       err = do_ovl_rename(&ovlrd, &list);
> +
>         ovl_revert_creds(old_cred);
>         if (update_nlink)
>                 ovl_nlink_end(new);
>         else
>                 ovl_drop_write(old);
> -out:
> +
>         dput(ovlrd.newdentry);
>         dput(ovlrd.olddentry);
>         dput(ovlrd.opaquedir);
> +out:
>         ovl_cache_free(&list);
>         return err;
> -
> -out_unlock:
> -       unlock_rename(new_upperdir, old_upperdir);
> -       goto out_revert_creds;
>  }
>
>  static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
>
> --
> 2.47.3
>

Re: [PATCH v3 33/42] ovl: introduce struct ovl_renamedata

[Amir Goldstein]

On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Add a struct ovl_renamedata to group rename-related state that was
> previously stored in local variables. Embedd struct renamedata directly
> aligning with the vfs.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/dir.c | 123 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 86b72bf87833..052929b9b99d 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1090,6 +1090,15 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
>         return err;
>  }
>
> +struct ovl_renamedata {
> +       struct renamedata;
> +       struct dentry *opaquedir;
> +       struct dentry *olddentry;
> +       struct dentry *newdentry;
> +       bool cleanup_whiteout;
> +       bool overwrite;
> +};
> +

It's very clever to use fms extensions here
However, considering the fact that Neil's patch
https://lore.kernel.org/linux-fsdevel/20251113002050.676694-11-neilb@ownmail.net/
creates and uses ovl_do_rename_rd(), it might be better to use separate
struct renamedata *rd, ovl_rename_ctx *ctx
unless fms extensions have a way to refer to the embedded struct?

Speaking of Neil's patch set, I did a test merge and ovl_rename() was the
only visible conflict.

Most of the conflict is about the conflicting different refactoring prior to
the actual start/end_renaming() change.

I think if you guys can agree on a common prereq refactoring patch
that would make life easier for both patch sets.

I can give it a shot at providing the common patch.

Thanks,
Amir.

Re: [PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Miklos Szeredi]

On Thu, 13 Nov 2025 at 22:33, Christian Brauner <brauner@kernel.org> wrote:
>
> Extract the code that runs under overridden credentials into a separate
> do_ovl_rename() helper function. Error handling is simplified. The

Hmm, it's getting confusing between ovl_do_rename() and
do_ovl_rename().   Also I'd prefer not to lose ovl_ as /the/ prefix
unless absolutely necessary.

Re: [PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Amir Goldstein]

On Fri, Nov 14, 2025 at 10:17 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 13 Nov 2025 at 22:33, Christian Brauner <brauner@kernel.org> wrote:
> >
> > Extract the code that runs under overridden credentials into a separate
> > do_ovl_rename() helper function. Error handling is simplified. The
>
> Hmm, it's getting confusing between ovl_do_rename() and
> do_ovl_rename().   Also I'd prefer not to lose ovl_ as /the/ prefix
> unless absolutely necessary.

I was just thinking the same thing as I was trying to refactor
ovl_rename() to ovl_rename_start(); (something); ovl_rename_end()
I will try to think of a better convention.

BTW, we also have do_ovl_get_acl() which has managed to escape
the ovl_ namespace and has the same confusion with the ovl_do_ bunch.

Thanks,
Amir.

Re: [PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Christian Brauner]

On Fri, Nov 14, 2025 at 10:03:56AM +0100, Amir Goldstein wrote:
> On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Extract the code that runs under overridden credentials into a separate
> > do_ovl_rename() helper function. Error handling is simplified. The
> > helper returns errors directly instead of using goto labels.
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> 
> For the record, the only way I could review this patch is by manually
> moving the helper and doing diff, so while I approve the code
> I think this is unreviewable as it is posted.

This function should've never been allowed to be this large in the first
place. It is a giant pain to read and modify.

If you have an approach where you can refactor it in tiny steps that I
can incorporate, be my guest. The only reason I'm doing it is because it
gets in my way here.

I've already split the cleanup into two patches.

Re: [PATCH v3 33/42] ovl: introduce struct ovl_renamedata

[Amir Goldstein]

On Fri, Nov 14, 2025 at 10:04 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Add a struct ovl_renamedata to group rename-related state that was
> > previously stored in local variables. Embedd struct renamedata directly
> > aligning with the vfs.
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/overlayfs/dir.c | 123 +++++++++++++++++++++++++++++------------------------
> >  1 file changed, 68 insertions(+), 55 deletions(-)
> >
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index 86b72bf87833..052929b9b99d 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -1090,6 +1090,15 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
> >         return err;
> >  }
> >
> > +struct ovl_renamedata {
> > +       struct renamedata;
> > +       struct dentry *opaquedir;
> > +       struct dentry *olddentry;
> > +       struct dentry *newdentry;
> > +       bool cleanup_whiteout;
> > +       bool overwrite;
> > +};
> > +
>
> It's very clever to use fms extensions here
> However, considering the fact that Neil's patch
> https://lore.kernel.org/linux-fsdevel/20251113002050.676694-11-neilb@ownmail.net/
> creates and uses ovl_do_rename_rd(), it might be better to use separate
> struct renamedata *rd, ovl_rename_ctx *ctx
> unless fms extensions have a way to refer to the embedded struct?
>

Doh, I really got confused.
The dentries in ovl_renamedata are ovl dentries and the entries in
renamedata passed to ovl_do_rename_rd() are real dentries.
So forget what I said.

Thanks,
Amir.

Re: [PATCH v3 34/42] ovl: extract do_ovl_rename() helper function

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

On Fri, Nov 14, 2025 at 11:21 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Nov 14, 2025 at 10:03:56AM +0100, Amir Goldstein wrote:
> > On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Extract the code that runs under overridden credentials into a separate
> > > do_ovl_rename() helper function. Error handling is simplified. The
> > > helper returns errors directly instead of using goto labels.
> > >
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> >
> > For the record, the only way I could review this patch is by manually
> > moving the helper and doing diff, so while I approve the code
> > I think this is unreviewable as it is posted.
>
> This function should've never been allowed to be this large in the first
> place. It is a giant pain to read and modify.
>
> If you have an approach where you can refactor it in tiny steps that I
> can incorporate, be my guest. The only reason I'm doing it is because it
> gets in my way here.

Here you go

large function split into ovl_rename_start() ovl_rename_upper() ovl_rename_end()
this is clear, smallish patch and conforming to other overlayfs code.

Also tested that together with your new patches and all tests still pass.

Thanks,
Amir.

[-- Attachment #2: 0001-ovl-refactor-ovl_rename.patch --]
[-- Type: text/x-patch, Size: 11151 bytes --]

From 76bb2391d769e06cf0fe6ed1b4ef6a87c97f3290 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Thu, 13 Nov 2025 22:09:45 +0100
Subject: [PATCH] ovl: refactor ovl_rename()

Extract the code that runs under overridden credentials into a separate
ovl_rename_upper() helper function and the code that runs before and
after to ovl_rename_{start,end}(). Error handling is simplified. The
helpers returns errors directly instead of using goto labels.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 052929b9b99d1..f6d9d568e9440 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1096,109 +1096,101 @@ struct ovl_renamedata {
 	struct dentry *olddentry;
 	struct dentry *newdentry;
 	bool cleanup_whiteout;
+	bool update_nlink;
 	bool overwrite;
 };
 
-static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
-		      struct dentry *old, struct inode *newdir,
-		      struct dentry *new, unsigned int flags)
+static int ovl_rename_start(struct ovl_renamedata *ovlrd, struct list_head *list)
 {
-	int err;
-	struct dentry *old_upperdir;
-	struct dentry *new_upperdir;
-	struct dentry *trap, *de;
-	bool old_opaque;
-	bool new_opaque;
-	bool update_nlink = false;
+	struct dentry *old = ovlrd->old_dentry;
+	struct dentry *new = ovlrd->new_dentry;
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	const struct cred *old_cred = NULL;
-	struct ovl_fs *ofs = OVL_FS(old->d_sb);
-	struct ovl_renamedata ovlrd = {
-		.old_parent		= old->d_parent,
-		.old_dentry		= old,
-		.new_parent		= new->d_parent,
-		.new_dentry		= new,
-		.flags			= flags,
-		.cleanup_whiteout	= false,
-		.overwrite		= !(flags & RENAME_EXCHANGE),
-	};
-	LIST_HEAD(list);
-	bool samedir = ovlrd.old_parent == ovlrd.new_parent;
+	int err;
 
-	err = -EINVAL;
-	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
-		goto out;
+	if (ovlrd->flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
+		return -EINVAL;
 
-	ovlrd.flags &= ~RENAME_NOREPLACE;
+	ovlrd->flags &= ~RENAME_NOREPLACE;
 
 	/* Don't copy up directory trees */
 	err = -EXDEV;
 	if (!ovl_can_move(old))
-		goto out;
-	if (!ovlrd.overwrite && !ovl_can_move(new))
-		goto out;
+		return err;
+	if (!ovlrd->overwrite && !ovl_can_move(new))
+		return err;
 
-	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
-		err = ovl_check_empty_dir(new, &list);
+	if (ovlrd->overwrite && new_is_dir && !ovl_pure_upper(new)) {
+		err = ovl_check_empty_dir(new, list);
 		if (err)
-			goto out;
+			return err;
 	}
 
-	if (ovlrd.overwrite) {
+	if (ovlrd->overwrite) {
 		if (ovl_lower_positive(old)) {
 			if (!ovl_dentry_is_whiteout(new)) {
 				/* Whiteout source */
-				ovlrd.flags |= RENAME_WHITEOUT;
+				ovlrd->flags |= RENAME_WHITEOUT;
 			} else {
 				/* Switch whiteouts */
-				ovlrd.flags |= RENAME_EXCHANGE;
+				ovlrd->flags |= RENAME_EXCHANGE;
 			}
 		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
-			ovlrd.flags |= RENAME_EXCHANGE;
-			ovlrd.cleanup_whiteout = true;
+			ovlrd->flags |= RENAME_EXCHANGE;
+			ovlrd->cleanup_whiteout = true;
 		}
 	}
 
 	err = ovl_copy_up(old);
 	if (err)
-		goto out;
+		return err;
 
-	err = ovl_copy_up(ovlrd.new_parent);
+	err = ovl_copy_up(new->d_parent);
 	if (err)
-		goto out;
-	if (!ovlrd.overwrite) {
+		return err;
+	if (!ovlrd->overwrite) {
 		err = ovl_copy_up(new);
 		if (err)
-			goto out;
+			return err;
 	} else if (d_inode(new)) {
 		err = ovl_nlink_start(new);
 		if (err)
-			goto out;
+			return err;
 
-		update_nlink = true;
+		ovlrd->update_nlink = true;
 	}
 
-	if (!update_nlink) {
+	if (!ovlrd->update_nlink) {
 		/* ovl_nlink_start() took ovl_want_write() */
 		err = ovl_want_write(old);
 		if (err)
-			goto out;
+			return err;
 	}
 
-	old_cred = ovl_override_creds(old->d_sb);
+	return 0;
+}
 
-	if (!list_empty(&list)) {
-		ovlrd.opaquedir = ovl_clear_empty(new, &list);
-		err = PTR_ERR(ovlrd.opaquedir);
-		if (IS_ERR(ovlrd.opaquedir)) {
-			ovlrd.opaquedir = NULL;
-			goto out_revert_creds;
-		}
-	}
+static int ovl_rename_upper(struct ovl_renamedata *ovlrd, struct list_head *list)
+{
+	struct dentry *old = ovlrd->old_dentry;
+	struct dentry *new = ovlrd->new_dentry;
+	struct ovl_fs *ofs = OVL_FS(old->d_sb);
+	unsigned int flags = ovlrd->flags;
+	struct dentry *old_upperdir = ovl_dentry_upper(ovlrd->old_parent);
+	struct dentry *new_upperdir = ovl_dentry_upper(ovlrd->new_parent);
+	bool samedir = ovlrd->old_parent == ovlrd->new_parent;
+	bool is_dir = d_is_dir(old);
+	bool new_is_dir = d_is_dir(new);
+	struct dentry *trap, *de;
+	bool old_opaque, new_opaque;
+	int err;
 
-	old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
-	new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
+	if (!list_empty(list)) {
+		de = ovl_clear_empty(new, list);
+		if (IS_ERR(de))
+			return PTR_ERR(de);
+		ovlrd->opaquedir = de;
+	}
 
 	if (!samedir) {
 		/*
@@ -1208,32 +1200,30 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		 * lookup the origin inodes of the entries to fill d_ino.
 		 */
 		if (ovl_type_origin(old)) {
-			err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
+			err = ovl_set_impure(ovlrd->new_parent, new_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
-		if (!ovlrd.overwrite && ovl_type_origin(new)) {
-			err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
+		if (!ovlrd->overwrite && ovl_type_origin(new)) {
+			err = ovl_set_impure(ovlrd->old_parent, old_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
 	}
 
 	trap = lock_rename(new_upperdir, old_upperdir);
-	if (IS_ERR(trap)) {
-		err = PTR_ERR(trap);
-		goto out_revert_creds;
-	}
+	if (IS_ERR(trap))
+		return PTR_ERR(trap);
 
 	de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir,
 			      old->d_name.len);
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.olddentry = de;
+	ovlrd->olddentry = de;
 
 	err = -ESTALE;
-	if (!ovl_matches_upper(old, ovlrd.olddentry))
+	if (!ovl_matches_upper(old, ovlrd->olddentry))
 		goto out_unlock;
 
 	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
@@ -1241,73 +1231,74 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.newdentry = de;
+	ovlrd->newdentry = de;
 
 	old_opaque = ovl_dentry_is_opaque(old);
 	new_opaque = ovl_dentry_is_opaque(new);
 
 	err = -ESTALE;
 	if (d_inode(new) && ovl_dentry_upper(new)) {
-		if (ovlrd.opaquedir) {
-			if (ovlrd.newdentry != ovlrd.opaquedir)
+		if (ovlrd->opaquedir) {
+			if (ovlrd->newdentry != ovlrd->opaquedir)
 				goto out_unlock;
 		} else {
-			if (!ovl_matches_upper(new, ovlrd.newdentry))
+			if (!ovl_matches_upper(new, ovlrd->newdentry))
 				goto out_unlock;
 		}
 	} else {
-		if (!d_is_negative(ovlrd.newdentry)) {
-			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.newdentry))
+		if (!d_is_negative(ovlrd->newdentry)) {
+			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd->newdentry))
 				goto out_unlock;
 		} else {
-			if (ovlrd.flags & RENAME_EXCHANGE)
+			if (flags & RENAME_EXCHANGE)
 				goto out_unlock;
 		}
 	}
 
-	if (ovlrd.olddentry == trap)
+	if (ovlrd->olddentry == trap)
 		goto out_unlock;
-	if (ovlrd.newdentry == trap)
+	if (ovlrd->newdentry == trap)
 		goto out_unlock;
 
-	if (ovlrd.olddentry->d_inode == ovlrd.newdentry->d_inode)
+	if (ovlrd->olddentry->d_inode == ovlrd->newdentry->d_inode)
 		goto out_unlock;
 
 	err = 0;
 	if (ovl_type_merge_or_lower(old))
 		err = ovl_set_redirect(old, samedir);
-	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
-		err = ovl_set_opaque_xerr(old, ovlrd.olddentry, -EXDEV);
+	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd->new_parent))
+		err = ovl_set_opaque_xerr(old, ovlrd->olddentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
+	if (!ovlrd->overwrite && ovl_type_merge_or_lower(new))
 		err = ovl_set_redirect(new, samedir);
-	else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
-		 ovl_type_merge(ovlrd.old_parent))
-		err = ovl_set_opaque_xerr(new, ovlrd.newdentry, -EXDEV);
+	else if (!ovlrd->overwrite && new_is_dir && !new_opaque &&
+		 ovl_type_merge(ovlrd->old_parent))
+		err = ovl_set_opaque_xerr(new, ovlrd->newdentry, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename(ofs, old_upperdir, ovlrd.olddentry,
-			    new_upperdir, ovlrd.newdentry, flags);
+	err = ovl_do_rename(ofs, old_upperdir, ovlrd->olddentry,
+			    new_upperdir, ovlrd->newdentry, flags);
+out_unlock:
 	unlock_rename(new_upperdir, old_upperdir);
 	if (err)
-		goto out_revert_creds;
+		return err;
 
-	if (ovlrd.cleanup_whiteout)
-		ovl_cleanup(ofs, old_upperdir, ovlrd.newdentry);
+	if (ovlrd->cleanup_whiteout)
+		ovl_cleanup(ofs, old_upperdir, ovlrd->newdentry);
 
-	if (ovlrd.overwrite && d_inode(new)) {
+	if (ovlrd->overwrite && d_inode(new)) {
 		if (new_is_dir)
 			clear_nlink(d_inode(new));
 		else
 			ovl_drop_nlink(new);
 	}
 
-	ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
-			 (!ovlrd.overwrite && ovl_type_origin(new)));
-	ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
+	ovl_dir_modified(ovlrd->old_parent, ovl_type_origin(old) ||
+			 (!ovlrd->overwrite && ovl_type_origin(new)));
+	ovl_dir_modified(ovlrd->new_parent, ovl_type_origin(old) ||
 			 (d_inode(new) && ovl_type_origin(new)));
 
 	/* copy ctime: */
@@ -1315,22 +1306,51 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (d_inode(new) && ovl_dentry_upper(new))
 		ovl_copyattr(d_inode(new));
 
-out_revert_creds:
-	ovl_revert_creds(old_cred);
-	if (update_nlink)
-		ovl_nlink_end(new);
+	return err;
+}
+
+static void ovl_rename_end(struct ovl_renamedata *ovlrd)
+{
+	if (ovlrd->update_nlink)
+		ovl_nlink_end(ovlrd->new_dentry);
 	else
-		ovl_drop_write(old);
+		ovl_drop_write(ovlrd->old_dentry);
+
+	dput(ovlrd->newdentry);
+	dput(ovlrd->olddentry);
+	dput(ovlrd->opaquedir);
+}
+
+static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
+		      struct dentry *old, struct inode *newdir,
+		      struct dentry *new, unsigned int flags)
+{
+	const struct cred *old_cred = NULL;
+	struct ovl_renamedata ovlrd = {
+		.old_parent		= old->d_parent,
+		.old_dentry		= old,
+		.new_parent		= new->d_parent,
+		.new_dentry		= new,
+		.flags			= flags,
+		.overwrite		= !(flags & RENAME_EXCHANGE),
+	};
+	LIST_HEAD(list);
+	int err;
+
+	err = ovl_rename_start(&ovlrd, &list);
+	if (err)
+		goto out;
+
+	old_cred = ovl_override_creds(old->d_sb);
+
+	err = ovl_rename_upper(&ovlrd, &list);
+
+	ovl_revert_creds(old_cred);
+
+	ovl_rename_end(&ovlrd);
 out:
-	dput(ovlrd.newdentry);
-	dput(ovlrd.olddentry);
-	dput(ovlrd.opaquedir);
 	ovl_cache_free(&list);
 	return err;
-
-out_unlock:
-	unlock_rename(new_upperdir, old_upperdir);
-	goto out_revert_creds;
 }
 
 static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
-- 
2.51.1

Re: [PATCH v3 40/42] ovl: refactor ovl_fill_super()

[Amir Goldstein]

On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Split the core into a separate helper in preparation of converting the
> caller to the scoped ovl cred guard.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/super.c | 91 +++++++++++++++++++++++++++-------------------------
>  1 file changed, 48 insertions(+), 43 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 43ee4c7296a7..e3781fccaef8 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1369,53 +1369,35 @@ static void ovl_set_d_op(struct super_block *sb)
>         set_default_d_op(sb, &ovl_dentry_operations);
>  }
>
> -int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> +static int do_ovl_fill_super(struct fs_context *fc, struct super_block *sb)
>  {
>         struct ovl_fs *ofs = sb->s_fs_info;
> +       struct cred *creator_cred = (struct cred *)ofs->creator_cred;
>         struct ovl_fs_context *ctx = fc->fs_private;
> -       const struct cred *old_cred = NULL;
> -       struct dentry *root_dentry;
> -       struct ovl_entry *oe;
>         struct ovl_layer *layers;
> -       struct cred *cred;
> +       struct ovl_entry *oe = NULL;
>         int err;
>
> -       err = -EIO;
> -       if (WARN_ON(fc->user_ns != current_user_ns()))
> -               goto out_err;
> -
> -       ovl_set_d_op(sb);
> -
> -       err = -ENOMEM;
> -       if (!ofs->creator_cred)
> -               ofs->creator_cred = cred = prepare_creds();
> -       else
> -               cred = (struct cred *)ofs->creator_cred;
> -       if (!cred)
> -               goto out_err;
> -
> -       old_cred = ovl_override_creds(sb);
> -
>         err = ovl_fs_params_verify(ctx, &ofs->config);
>         if (err)
> -               goto out_err;
> +               return err;
>
>         err = -EINVAL;
>         if (ctx->nr == 0) {
>                 if (!(fc->sb_flags & SB_SILENT))
>                         pr_err("missing 'lowerdir'\n");
> -               goto out_err;
> +               return err;
>         }
>
>         err = -ENOMEM;
>         layers = kcalloc(ctx->nr + 1, sizeof(struct ovl_layer), GFP_KERNEL);
>         if (!layers)
> -               goto out_err;
> +               return err;
>
>         ofs->config.lowerdirs = kcalloc(ctx->nr + 1, sizeof(char *), GFP_KERNEL);
>         if (!ofs->config.lowerdirs) {
>                 kfree(layers);
> -               goto out_err;
> +               return err;
>         }
>         ofs->layers = layers;
>         /*
> @@ -1448,12 +1430,12 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>                 err = -EINVAL;
>                 if (!ofs->config.workdir) {
>                         pr_err("missing 'workdir'\n");
> -                       goto out_err;
> +                       return err;
>                 }
>
>                 err = ovl_get_upper(sb, ofs, &layers[0], &ctx->upper);
>                 if (err)
> -                       goto out_err;
> +                       return err;
>
>                 upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
>                 if (!ovl_should_sync(ofs)) {
> @@ -1461,13 +1443,13 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>                         if (errseq_check(&upper_sb->s_wb_err, ofs->errseq)) {
>                                 err = -EIO;
>                                 pr_err("Cannot mount volatile when upperdir has an unseen error. Sync upperdir fs to clear state.\n");
> -                               goto out_err;
> +                               return err;
>                         }
>                 }
>
>                 err = ovl_get_workdir(sb, ofs, &ctx->upper, &ctx->work);
>                 if (err)
> -                       goto out_err;
> +                       return err;
>
>                 if (!ofs->workdir)
>                         sb->s_flags |= SB_RDONLY;
> @@ -1478,7 +1460,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>         oe = ovl_get_lowerstack(sb, ctx, ofs, layers);
>         err = PTR_ERR(oe);
>         if (IS_ERR(oe))
> -               goto out_err;
> +               return err;
>
>         /* If the upper fs is nonexistent, we mark overlayfs r/o too */
>         if (!ovl_upper_mnt(ofs))
> @@ -1531,7 +1513,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>                 sb->s_export_op = &ovl_export_fid_operations;
>
>         /* Never override disk quota limits or use reserved space */
> -       cap_lower(cred->cap_effective, CAP_SYS_RESOURCE);
> +       cap_lower(creator_cred->cap_effective, CAP_SYS_RESOURCE);
>
>         sb->s_magic = OVERLAYFS_SUPER_MAGIC;
>         sb->s_xattr = ovl_xattr_handlers(ofs);
> @@ -1549,27 +1531,50 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>         sb->s_iflags |= SB_I_EVM_HMAC_UNSUPPORTED;
>
>         err = -ENOMEM;
> -       root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
> -       if (!root_dentry)
> +       sb->s_root = ovl_get_root(sb, ctx->upper.dentry, oe);
> +       if (!sb->s_root)
>                 goto out_free_oe;
>
> -       sb->s_root = root_dentry;
> -
> -       ovl_revert_creds(old_cred);
>         return 0;
>
>  out_free_oe:
>         ovl_free_entry(oe);
> -out_err:
> -       /*
> -        * Revert creds before calling ovl_free_fs() which will call
> -        * put_cred() and put_cred() requires that the cred's that are
> -        * put are not the caller's creds, i.e., current->cred.
> -        */
> -       if (old_cred)
> +       return err;
> +}
> +
> +int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> +{
> +       struct ovl_fs *ofs = sb->s_fs_info;
> +       const struct cred *old_cred = NULL;
> +       struct cred *cred;
> +       int err;
> +
> +       err = -EIO;
> +       if (WARN_ON(fc->user_ns != current_user_ns()))
> +               goto out_err;
> +
> +       ovl_set_d_op(sb);
> +
> +       err = -ENOMEM;
> +       if (!ofs->creator_cred)
> +               ofs->creator_cred = cred = prepare_creds();
> +       else
> +               cred = (struct cred *)ofs->creator_cred;
> +       if (!cred)
> +               goto out_err;
> +
> +       old_cred = ovl_override_creds(sb);
> +
> +       err = do_ovl_fill_super(fc, sb);
> +
>         ovl_revert_creds(old_cred);
> +
> +out_err:
> +       if (err) {
>                 ovl_free_fs(ofs);
>                 sb->s_fs_info = NULL;
> +       }
> +
>         return err;
>  }
>
>
> --
> 2.47.3
>

Considering Miklos' complaint about do_ovl_ helpers, how about:

--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1369,10 +1369,10 @@ static void ovl_set_d_op(struct super_block *sb)
        set_default_d_op(sb, &ovl_dentry_operations);
 }

-static int do_ovl_fill_super(struct fs_context *fc, struct super_block *sb)
+static int ovl_fill_super_cred(struct fs_context *fc, struct super_block *sb,
+                              struct cred *creator_cred)
 {
        struct ovl_fs *ofs = sb->s_fs_info;
-       struct cred *creator_cred = (struct cred *)ofs->creator_cred;
        struct ovl_fs_context *ctx = fc->fs_private;
        struct ovl_layer *layers;
        struct ovl_entry *oe = NULL;
@@ -1545,6 +1545,7 @@ static int do_ovl_fill_super(struct fs_context
*fc, struct super_block *sb)
 int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 {
        struct ovl_fs *ofs = sb->s_fs_info;
+       struct cred *cred = (struct cred *)ofs->creator_cred;
        int err;

        err = -EIO;
@@ -1555,13 +1556,13 @@ int ovl_fill_super(struct super_block *sb,
struct fs_context *fc)

        if (!ofs->creator_cred) {
                err = -ENOMEM;
-               ofs->creator_cred = prepare_creds();
-               if (!ofs->creator_cred)
+               ofs->creator_cred = cred = prepare_creds();
+               if (!cred)
                        goto out_err;
        }

        with_ovl_creds(sb)
-               err = do_ovl_fill_super(fc, sb);
+               err = ovl_fill_super_cred(fc, sb, cred);

---

Which is also a bit more explicit about the fact that the helper
is modifying the creator_cred.

Thanks,
Amir.

Re: [PATCH v3 33/42] ovl: introduce struct ovl_renamedata

[Amir Goldstein]

On Fri, Nov 14, 2025 at 11:26 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 10:04 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Add a struct ovl_renamedata to group rename-related state that was
> > > previously stored in local variables. Embedd struct renamedata directly
> > > aligning with the vfs.
> > >
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > ---
> > >  fs/overlayfs/dir.c | 123 +++++++++++++++++++++++++++++------------------------
> > >  1 file changed, 68 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > > index 86b72bf87833..052929b9b99d 100644
> > > --- a/fs/overlayfs/dir.c
> > > +++ b/fs/overlayfs/dir.c
> > > @@ -1090,6 +1090,15 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
> > >         return err;
> > >  }
> > >
> > > +struct ovl_renamedata {
> > > +       struct renamedata;
> > > +       struct dentry *opaquedir;
> > > +       struct dentry *olddentry;
> > > +       struct dentry *newdentry;
> > > +       bool cleanup_whiteout;
> > > +       bool overwrite;
> > > +};
> > > +
> >
> > It's very clever to use fms extensions here
> > However, considering the fact that Neil's patch
> > https://lore.kernel.org/linux-fsdevel/20251113002050.676694-11-neilb@ownmail.net/
> > creates and uses ovl_do_rename_rd(), it might be better to use separate
> > struct renamedata *rd, ovl_rename_ctx *ctx
> > unless fms extensions have a way to refer to the embedded struct?
> >
>
> Doh, I really got confused.
> The dentries in ovl_renamedata are ovl dentries and the entries in
> renamedata passed to ovl_do_rename_rd() are real dentries.
> So forget what I said.

To help with mine (and others) confusion I think it would be better to
be explicit about upper vs. plain dentry in ovl functions where
both types exist. It's one of the easiest things to get wrong in ovl code:

struct ovl_renamedata {
       struct renamedata;
       struct dentry *opaquedir;
       struct dentry *old_upper;
       struct dentry *new_upper;
       bool cleanup_whiteout;
       bool overwrite;
};

IMO ovl_rename() was not doing a good job with 'old' vs. 'olddentry',
so for the conversion to ovl_renamedata, we should fix this misnomer.

Thanks,
Amir.

Re: [PATCH v3 33/42] ovl: introduce struct ovl_renamedata

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 3442 bytes --]

On Fri, Nov 14, 2025 at 2:01 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 11:26 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 10:04 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Thu, Nov 13, 2025 at 10:33 PM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > Add a struct ovl_renamedata to group rename-related state that was
> > > > previously stored in local variables. Embedd struct renamedata directly
> > > > aligning with the vfs.
> > > >
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > ---
> > > >  fs/overlayfs/dir.c | 123 +++++++++++++++++++++++++++++------------------------
> > > >  1 file changed, 68 insertions(+), 55 deletions(-)
> > > >
> > > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > > > index 86b72bf87833..052929b9b99d 100644
> > > > --- a/fs/overlayfs/dir.c
> > > > +++ b/fs/overlayfs/dir.c
> > > > @@ -1090,6 +1090,15 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
> > > >         return err;
> > > >  }
> > > >
> > > > +struct ovl_renamedata {
> > > > +       struct renamedata;
> > > > +       struct dentry *opaquedir;
> > > > +       struct dentry *olddentry;
> > > > +       struct dentry *newdentry;
> > > > +       bool cleanup_whiteout;
> > > > +       bool overwrite;
> > > > +};
> > > > +
> > >
> > > It's very clever to use fms extensions here
> > > However, considering the fact that Neil's patch
> > > https://lore.kernel.org/linux-fsdevel/20251113002050.676694-11-neilb@ownmail.net/
> > > creates and uses ovl_do_rename_rd(), it might be better to use separate
> > > struct renamedata *rd, ovl_rename_ctx *ctx
> > > unless fms extensions have a way to refer to the embedded struct?
> > >
> >
> > Doh, I really got confused.
> > The dentries in ovl_renamedata are ovl dentries and the entries in
> > renamedata passed to ovl_do_rename_rd() are real dentries.
> > So forget what I said.
>
> To help with mine (and others) confusion I think it would be better to
> be explicit about upper vs. plain dentry in ovl functions where
> both types exist. It's one of the easiest things to get wrong in ovl code:
>
> struct ovl_renamedata {
>        struct renamedata;
>        struct dentry *opaquedir;
>        struct dentry *old_upper;
>        struct dentry *new_upper;
>        bool cleanup_whiteout;
>        bool overwrite;
> };
>
> IMO ovl_rename() was not doing a good job with 'old' vs. 'olddentry',
> so for the conversion to ovl_renamedata, we should fix this misnomer.
>

Note that this struct had
ovlrd->new_dentry
ovlrd->old_dentry
and also
ovlrd->newdentry
ovlrd->olddentry

Which one was the ovl and which one is the upper???

It so happens that Neil's branch gets rid of those misnomers anyway,
so the change is only needed temporarily.

Here goes.
Attached the 3 patches, which include the members rename:

6bcfb42324149 ovl: port ovl_rename() to cred guard
23f364fc4ba6e ovl: refactor ovl_rename()
d6be8e418a8b ovl: introduce struct ovl_renamedata

With these changes, it was easier for me to merge with Neil's branch

Please find my merge resolution at:
https://github.com/amir73il/linux/commits/pdirops/

Which includes my 3 changes patches and it passed the sanity fstests.

Thanks,
Amir.

[-- Attachment #2: 0001-ovl-refactor_and_port-ovl_rename.patch --]
[-- Type: text/x-patch, Size: 21425 bytes --]

From d6be8e418a8b88dc1f7d8e98a1863a64fd05bca0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Thu, 13 Nov 2025 22:09:30 +0100
Subject: [PATCH 1/3] ovl: introduce struct ovl_renamedata

Add a struct ovl_renamedata to group rename-related state that was
previously stored in local variables. Embedd struct renamedata directly
aligning with the vfs.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 86b72bf87833f..b5b06ba9632a5 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1090,6 +1090,16 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
 	return err;
 }
 
+struct ovl_renamedata {
+	struct renamedata;
+	struct dentry *opaquedir;
+	struct dentry *old_upper;
+	struct dentry *new_upper;
+	bool cleanup_whiteout;
+	bool update_nlink;
+	bool overwrite;
+};
+
 static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		      struct dentry *old, struct inode *newdir,
 		      struct dentry *new, unsigned int flags)
@@ -1097,53 +1107,56 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	int err;
 	struct dentry *old_upperdir;
 	struct dentry *new_upperdir;
-	struct dentry *olddentry = NULL;
-	struct dentry *newdentry = NULL;
 	struct dentry *trap, *de;
 	bool old_opaque;
 	bool new_opaque;
-	bool cleanup_whiteout = false;
-	bool update_nlink = false;
-	bool overwrite = !(flags & RENAME_EXCHANGE);
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	bool samedir = olddir == newdir;
-	struct dentry *opaquedir = NULL;
 	const struct cred *old_cred = NULL;
 	struct ovl_fs *ofs = OVL_FS(old->d_sb);
+	struct ovl_renamedata ovlrd = {
+		.old_parent		= old->d_parent,
+		.old_dentry		= old,
+		.new_parent		= new->d_parent,
+		.new_dentry		= new,
+		.flags			= flags,
+		.cleanup_whiteout	= false,
+		.overwrite		= !(flags & RENAME_EXCHANGE),
+	};
 	LIST_HEAD(list);
+	bool samedir = ovlrd.old_parent == ovlrd.new_parent;
 
 	err = -EINVAL;
-	if (flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
+	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
 		goto out;
 
-	flags &= ~RENAME_NOREPLACE;
+	ovlrd.flags &= ~RENAME_NOREPLACE;
 
 	/* Don't copy up directory trees */
 	err = -EXDEV;
 	if (!ovl_can_move(old))
 		goto out;
-	if (!overwrite && !ovl_can_move(new))
+	if (!ovlrd.overwrite && !ovl_can_move(new))
 		goto out;
 
-	if (overwrite && new_is_dir && !ovl_pure_upper(new)) {
+	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
 		err = ovl_check_empty_dir(new, &list);
 		if (err)
 			goto out;
 	}
 
-	if (overwrite) {
+	if (ovlrd.overwrite) {
 		if (ovl_lower_positive(old)) {
 			if (!ovl_dentry_is_whiteout(new)) {
 				/* Whiteout source */
-				flags |= RENAME_WHITEOUT;
+				ovlrd.flags |= RENAME_WHITEOUT;
 			} else {
 				/* Switch whiteouts */
-				flags |= RENAME_EXCHANGE;
+				ovlrd.flags |= RENAME_EXCHANGE;
 			}
 		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
-			flags |= RENAME_EXCHANGE;
-			cleanup_whiteout = true;
+			ovlrd.flags |= RENAME_EXCHANGE;
+			ovlrd.cleanup_whiteout = true;
 		}
 	}
 
@@ -1151,10 +1164,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (err)
 		goto out;
 
-	err = ovl_copy_up(new->d_parent);
+	err = ovl_copy_up(ovlrd.new_parent);
 	if (err)
 		goto out;
-	if (!overwrite) {
+	if (!ovlrd.overwrite) {
 		err = ovl_copy_up(new);
 		if (err)
 			goto out;
@@ -1163,10 +1176,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		if (err)
 			goto out;
 
-		update_nlink = true;
+		ovlrd.update_nlink = true;
 	}
 
-	if (!update_nlink) {
+	if (!ovlrd.update_nlink) {
 		/* ovl_nlink_start() took ovl_want_write() */
 		err = ovl_want_write(old);
 		if (err)
@@ -1176,16 +1189,16 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	old_cred = ovl_override_creds(old->d_sb);
 
 	if (!list_empty(&list)) {
-		opaquedir = ovl_clear_empty(new, &list);
-		err = PTR_ERR(opaquedir);
-		if (IS_ERR(opaquedir)) {
-			opaquedir = NULL;
+		ovlrd.opaquedir = ovl_clear_empty(new, &list);
+		err = PTR_ERR(ovlrd.opaquedir);
+		if (IS_ERR(ovlrd.opaquedir)) {
+			ovlrd.opaquedir = NULL;
 			goto out_revert_creds;
 		}
 	}
 
-	old_upperdir = ovl_dentry_upper(old->d_parent);
-	new_upperdir = ovl_dentry_upper(new->d_parent);
+	old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
+	new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
 
 	if (!samedir) {
 		/*
@@ -1195,12 +1208,12 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		 * lookup the origin inodes of the entries to fill d_ino.
 		 */
 		if (ovl_type_origin(old)) {
-			err = ovl_set_impure(new->d_parent, new_upperdir);
+			err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
 			if (err)
 				goto out_revert_creds;
 		}
-		if (!overwrite && ovl_type_origin(new)) {
-			err = ovl_set_impure(old->d_parent, old_upperdir);
+		if (!ovlrd.overwrite && ovl_type_origin(new)) {
+			err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
 			if (err)
 				goto out_revert_creds;
 		}
@@ -1217,10 +1230,10 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	olddentry = de;
+	ovlrd.old_upper = de;
 
 	err = -ESTALE;
-	if (!ovl_matches_upper(old, olddentry))
+	if (!ovl_matches_upper(old, ovlrd.old_upper))
 		goto out_unlock;
 
 	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
@@ -1228,73 +1241,73 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	newdentry = de;
+	ovlrd.new_upper = de;
 
 	old_opaque = ovl_dentry_is_opaque(old);
 	new_opaque = ovl_dentry_is_opaque(new);
 
 	err = -ESTALE;
 	if (d_inode(new) && ovl_dentry_upper(new)) {
-		if (opaquedir) {
-			if (newdentry != opaquedir)
+		if (ovlrd.opaquedir) {
+			if (ovlrd.new_upper != ovlrd.opaquedir)
 				goto out_unlock;
 		} else {
-			if (!ovl_matches_upper(new, newdentry))
+			if (!ovl_matches_upper(new, ovlrd.new_upper))
 				goto out_unlock;
 		}
 	} else {
-		if (!d_is_negative(newdentry)) {
-			if (!new_opaque || !ovl_upper_is_whiteout(ofs, newdentry))
+		if (!d_is_negative(ovlrd.new_upper)) {
+			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.new_upper))
 				goto out_unlock;
 		} else {
-			if (flags & RENAME_EXCHANGE)
+			if (ovlrd.flags & RENAME_EXCHANGE)
 				goto out_unlock;
 		}
 	}
 
-	if (olddentry == trap)
+	if (ovlrd.old_upper == trap)
 		goto out_unlock;
-	if (newdentry == trap)
+	if (ovlrd.new_upper == trap)
 		goto out_unlock;
 
-	if (olddentry->d_inode == newdentry->d_inode)
+	if (ovlrd.old_upper->d_inode == ovlrd.new_upper->d_inode)
 		goto out_unlock;
 
 	err = 0;
 	if (ovl_type_merge_or_lower(old))
 		err = ovl_set_redirect(old, samedir);
-	else if (is_dir && !old_opaque && ovl_type_merge(new->d_parent))
-		err = ovl_set_opaque_xerr(old, olddentry, -EXDEV);
+	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
+		err = ovl_set_opaque_xerr(old, ovlrd.old_upper, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	if (!overwrite && ovl_type_merge_or_lower(new))
+	if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
 		err = ovl_set_redirect(new, samedir);
-	else if (!overwrite && new_is_dir && !new_opaque &&
-		 ovl_type_merge(old->d_parent))
-		err = ovl_set_opaque_xerr(new, newdentry, -EXDEV);
+	else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
+		 ovl_type_merge(ovlrd.old_parent))
+		err = ovl_set_opaque_xerr(new, ovlrd.new_upper, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename(ofs, old_upperdir, olddentry,
-			    new_upperdir, newdentry, flags);
+	err = ovl_do_rename(ofs, old_upperdir, ovlrd.old_upper,
+			    new_upperdir, ovlrd.new_upper, flags);
 	unlock_rename(new_upperdir, old_upperdir);
 	if (err)
 		goto out_revert_creds;
 
-	if (cleanup_whiteout)
-		ovl_cleanup(ofs, old_upperdir, newdentry);
+	if (ovlrd.cleanup_whiteout)
+		ovl_cleanup(ofs, old_upperdir, ovlrd.new_upper);
 
-	if (overwrite && d_inode(new)) {
+	if (ovlrd.overwrite && d_inode(new)) {
 		if (new_is_dir)
 			clear_nlink(d_inode(new));
 		else
 			ovl_drop_nlink(new);
 	}
 
-	ovl_dir_modified(old->d_parent, ovl_type_origin(old) ||
-			 (!overwrite && ovl_type_origin(new)));
-	ovl_dir_modified(new->d_parent, ovl_type_origin(old) ||
+	ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
+			 (!ovlrd.overwrite && ovl_type_origin(new)));
+	ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
 			 (d_inode(new) && ovl_type_origin(new)));
 
 	/* copy ctime: */
@@ -1304,14 +1317,14 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 
 out_revert_creds:
 	ovl_revert_creds(old_cred);
-	if (update_nlink)
+	if (ovlrd.update_nlink)
 		ovl_nlink_end(new);
 	else
 		ovl_drop_write(old);
 out:
-	dput(newdentry);
-	dput(olddentry);
-	dput(opaquedir);
+	dput(ovlrd.new_upper);
+	dput(ovlrd.old_upper);
+	dput(ovlrd.opaquedir);
 	ovl_cache_free(&list);
 	return err;
 
-- 
2.51.1


From 23f364fc4ba6ee7cbd50d252f9d94c54f5d7e793 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 14 Nov 2025 16:54:03 +0100
Subject: [PATCH 2/3] ovl: refactor ovl_rename()

Extract the code that runs under overridden credentials into a separate
ovl_rename_upper() helper function and the code that runs before/after to
ovl_rename_start/end(). Error handling is simplified.
The helpers returns errors directly instead of using goto labels.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index b5b06ba9632a5..1baab90889167 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1100,105 +1100,97 @@ struct ovl_renamedata {
 	bool overwrite;
 };
 
-static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
-		      struct dentry *old, struct inode *newdir,
-		      struct dentry *new, unsigned int flags)
+static int ovl_rename_start(struct ovl_renamedata *ovlrd, struct list_head *list)
 {
-	int err;
-	struct dentry *old_upperdir;
-	struct dentry *new_upperdir;
-	struct dentry *trap, *de;
-	bool old_opaque;
-	bool new_opaque;
+	struct dentry *old = ovlrd->old_dentry;
+	struct dentry *new = ovlrd->new_dentry;
 	bool is_dir = d_is_dir(old);
 	bool new_is_dir = d_is_dir(new);
-	const struct cred *old_cred = NULL;
-	struct ovl_fs *ofs = OVL_FS(old->d_sb);
-	struct ovl_renamedata ovlrd = {
-		.old_parent		= old->d_parent,
-		.old_dentry		= old,
-		.new_parent		= new->d_parent,
-		.new_dentry		= new,
-		.flags			= flags,
-		.cleanup_whiteout	= false,
-		.overwrite		= !(flags & RENAME_EXCHANGE),
-	};
-	LIST_HEAD(list);
-	bool samedir = ovlrd.old_parent == ovlrd.new_parent;
+	int err;
 
-	err = -EINVAL;
-	if (ovlrd.flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
-		goto out;
+	if (ovlrd->flags & ~(RENAME_EXCHANGE | RENAME_NOREPLACE))
+		return -EINVAL;
 
-	ovlrd.flags &= ~RENAME_NOREPLACE;
+	ovlrd->flags &= ~RENAME_NOREPLACE;
 
 	/* Don't copy up directory trees */
 	err = -EXDEV;
 	if (!ovl_can_move(old))
-		goto out;
-	if (!ovlrd.overwrite && !ovl_can_move(new))
-		goto out;
+		return err;
+	if (!ovlrd->overwrite && !ovl_can_move(new))
+		return err;
 
-	if (ovlrd.overwrite && new_is_dir && !ovl_pure_upper(new)) {
-		err = ovl_check_empty_dir(new, &list);
+	if (ovlrd->overwrite && new_is_dir && !ovl_pure_upper(new)) {
+		err = ovl_check_empty_dir(new, list);
 		if (err)
-			goto out;
+			return err;
 	}
 
-	if (ovlrd.overwrite) {
+	if (ovlrd->overwrite) {
 		if (ovl_lower_positive(old)) {
 			if (!ovl_dentry_is_whiteout(new)) {
 				/* Whiteout source */
-				ovlrd.flags |= RENAME_WHITEOUT;
+				ovlrd->flags |= RENAME_WHITEOUT;
 			} else {
 				/* Switch whiteouts */
-				ovlrd.flags |= RENAME_EXCHANGE;
+				ovlrd->flags |= RENAME_EXCHANGE;
 			}
 		} else if (is_dir && ovl_dentry_is_whiteout(new)) {
-			ovlrd.flags |= RENAME_EXCHANGE;
-			ovlrd.cleanup_whiteout = true;
+			ovlrd->flags |= RENAME_EXCHANGE;
+			ovlrd->cleanup_whiteout = true;
 		}
 	}
 
 	err = ovl_copy_up(old);
 	if (err)
-		goto out;
+		return err;
 
-	err = ovl_copy_up(ovlrd.new_parent);
+	err = ovl_copy_up(new->d_parent);
 	if (err)
-		goto out;
-	if (!ovlrd.overwrite) {
+		return err;
+	if (!ovlrd->overwrite) {
 		err = ovl_copy_up(new);
 		if (err)
-			goto out;
+			return err;
 	} else if (d_inode(new)) {
 		err = ovl_nlink_start(new);
 		if (err)
-			goto out;
+			return err;
 
-		ovlrd.update_nlink = true;
+		ovlrd->update_nlink = true;
 	}
 
-	if (!ovlrd.update_nlink) {
+	if (!ovlrd->update_nlink) {
 		/* ovl_nlink_start() took ovl_want_write() */
 		err = ovl_want_write(old);
 		if (err)
-			goto out;
+			return err;
 	}
 
-	old_cred = ovl_override_creds(old->d_sb);
+	return 0;
+}
 
-	if (!list_empty(&list)) {
-		ovlrd.opaquedir = ovl_clear_empty(new, &list);
-		err = PTR_ERR(ovlrd.opaquedir);
-		if (IS_ERR(ovlrd.opaquedir)) {
-			ovlrd.opaquedir = NULL;
-			goto out_revert_creds;
-		}
-	}
+static int ovl_rename_upper(struct ovl_renamedata *ovlrd, struct list_head *list)
+{
+	struct dentry *old = ovlrd->old_dentry;
+	struct dentry *new = ovlrd->new_dentry;
+	struct ovl_fs *ofs = OVL_FS(old->d_sb);
+	unsigned int flags = ovlrd->flags;
+	struct dentry *old_upperdir = ovl_dentry_upper(ovlrd->old_parent);
+	struct dentry *new_upperdir = ovl_dentry_upper(ovlrd->new_parent);
+	bool samedir = ovlrd->old_parent == ovlrd->new_parent;
+	bool is_dir = d_is_dir(old);
+	bool new_is_dir = d_is_dir(new);
+	struct dentry *trap, *de;
+	bool old_opaque, new_opaque;
+	int err;
 
-	old_upperdir = ovl_dentry_upper(ovlrd.old_parent);
-	new_upperdir = ovl_dentry_upper(ovlrd.new_parent);
+	if (!list_empty(list)) {
+		de = ovl_clear_empty(new, list);
+		if (IS_ERR(de))
+			return PTR_ERR(de);
+		ovlrd->opaquedir = de;
+	}
 
 	if (!samedir) {
 		/*
@@ -1208,32 +1200,30 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		 * lookup the origin inodes of the entries to fill d_ino.
 		 */
 		if (ovl_type_origin(old)) {
-			err = ovl_set_impure(ovlrd.new_parent, new_upperdir);
+			err = ovl_set_impure(ovlrd->new_parent, new_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
-		if (!ovlrd.overwrite && ovl_type_origin(new)) {
-			err = ovl_set_impure(ovlrd.old_parent, old_upperdir);
+		if (!ovlrd->overwrite && ovl_type_origin(new)) {
+			err = ovl_set_impure(ovlrd->old_parent, old_upperdir);
 			if (err)
-				goto out_revert_creds;
+				return err;
 		}
 	}
 
 	trap = lock_rename(new_upperdir, old_upperdir);
-	if (IS_ERR(trap)) {
-		err = PTR_ERR(trap);
-		goto out_revert_creds;
-	}
+	if (IS_ERR(trap))
+		return PTR_ERR(trap);
 
 	de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir,
 			      old->d_name.len);
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.old_upper = de;
+	ovlrd->old_upper = de;
 
 	err = -ESTALE;
-	if (!ovl_matches_upper(old, ovlrd.old_upper))
+	if (!ovl_matches_upper(old, ovlrd->old_upper))
 		goto out_unlock;
 
 	de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir,
@@ -1241,73 +1231,74 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	err = PTR_ERR(de);
 	if (IS_ERR(de))
 		goto out_unlock;
-	ovlrd.new_upper = de;
+	ovlrd->new_upper = de;
 
 	old_opaque = ovl_dentry_is_opaque(old);
 	new_opaque = ovl_dentry_is_opaque(new);
 
 	err = -ESTALE;
 	if (d_inode(new) && ovl_dentry_upper(new)) {
-		if (ovlrd.opaquedir) {
-			if (ovlrd.new_upper != ovlrd.opaquedir)
+		if (ovlrd->opaquedir) {
+			if (ovlrd->new_upper != ovlrd->opaquedir)
 				goto out_unlock;
 		} else {
-			if (!ovl_matches_upper(new, ovlrd.new_upper))
+			if (!ovl_matches_upper(new, ovlrd->new_upper))
 				goto out_unlock;
 		}
 	} else {
-		if (!d_is_negative(ovlrd.new_upper)) {
-			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd.new_upper))
+		if (!d_is_negative(ovlrd->new_upper)) {
+			if (!new_opaque || !ovl_upper_is_whiteout(ofs, ovlrd->new_upper))
 				goto out_unlock;
 		} else {
-			if (ovlrd.flags & RENAME_EXCHANGE)
+			if (flags & RENAME_EXCHANGE)
 				goto out_unlock;
 		}
 	}
 
-	if (ovlrd.old_upper == trap)
+	if (ovlrd->old_upper == trap)
 		goto out_unlock;
-	if (ovlrd.new_upper == trap)
+	if (ovlrd->new_upper == trap)
 		goto out_unlock;
 
-	if (ovlrd.old_upper->d_inode == ovlrd.new_upper->d_inode)
+	if (ovlrd->old_upper->d_inode == ovlrd->new_upper->d_inode)
 		goto out_unlock;
 
 	err = 0;
 	if (ovl_type_merge_or_lower(old))
 		err = ovl_set_redirect(old, samedir);
-	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd.new_parent))
-		err = ovl_set_opaque_xerr(old, ovlrd.old_upper, -EXDEV);
+	else if (is_dir && !old_opaque && ovl_type_merge(ovlrd->new_parent))
+		err = ovl_set_opaque_xerr(old, ovlrd->old_upper, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	if (!ovlrd.overwrite && ovl_type_merge_or_lower(new))
+	if (!ovlrd->overwrite && ovl_type_merge_or_lower(new))
 		err = ovl_set_redirect(new, samedir);
-	else if (!ovlrd.overwrite && new_is_dir && !new_opaque &&
-		 ovl_type_merge(ovlrd.old_parent))
-		err = ovl_set_opaque_xerr(new, ovlrd.new_upper, -EXDEV);
+	else if (!ovlrd->overwrite && new_is_dir && !new_opaque &&
+		 ovl_type_merge(ovlrd->old_parent))
+		err = ovl_set_opaque_xerr(new, ovlrd->new_upper, -EXDEV);
 	if (err)
 		goto out_unlock;
 
-	err = ovl_do_rename(ofs, old_upperdir, ovlrd.old_upper,
-			    new_upperdir, ovlrd.new_upper, flags);
+	err = ovl_do_rename(ofs, old_upperdir, ovlrd->old_upper,
+			    new_upperdir, ovlrd->new_upper, flags);
+out_unlock:
 	unlock_rename(new_upperdir, old_upperdir);
 	if (err)
-		goto out_revert_creds;
+		return err;
 
-	if (ovlrd.cleanup_whiteout)
-		ovl_cleanup(ofs, old_upperdir, ovlrd.new_upper);
+	if (ovlrd->cleanup_whiteout)
+		ovl_cleanup(ofs, old_upperdir, ovlrd->new_upper);
 
-	if (ovlrd.overwrite && d_inode(new)) {
+	if (ovlrd->overwrite && d_inode(new)) {
 		if (new_is_dir)
 			clear_nlink(d_inode(new));
 		else
 			ovl_drop_nlink(new);
 	}
 
-	ovl_dir_modified(ovlrd.old_parent, ovl_type_origin(old) ||
-			 (!ovlrd.overwrite && ovl_type_origin(new)));
-	ovl_dir_modified(ovlrd.new_parent, ovl_type_origin(old) ||
+	ovl_dir_modified(ovlrd->old_parent, ovl_type_origin(old) ||
+			 (!ovlrd->overwrite && ovl_type_origin(new)));
+	ovl_dir_modified(ovlrd->new_parent, ovl_type_origin(old) ||
 			 (d_inode(new) && ovl_type_origin(new)));
 
 	/* copy ctime: */
@@ -1315,22 +1306,49 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (d_inode(new) && ovl_dentry_upper(new))
 		ovl_copyattr(d_inode(new));
 
-out_revert_creds:
-	ovl_revert_creds(old_cred);
-	if (ovlrd.update_nlink)
-		ovl_nlink_end(new);
+	return err;
+}
+
+static void ovl_rename_end(struct ovl_renamedata *ovlrd)
+{
+	if (ovlrd->update_nlink)
+		ovl_nlink_end(ovlrd->new_dentry);
 	else
-		ovl_drop_write(old);
+		ovl_drop_write(ovlrd->old_dentry);
+}
+
+static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
+		      struct dentry *old, struct inode *newdir,
+		      struct dentry *new, unsigned int flags)
+{
+	const struct cred *old_cred = NULL;
+	struct ovl_renamedata ovlrd = {
+		.old_parent		= old->d_parent,
+		.old_dentry		= old,
+		.new_parent		= new->d_parent,
+		.new_dentry		= new,
+		.flags			= flags,
+		.overwrite		= !(flags & RENAME_EXCHANGE),
+	};
+	LIST_HEAD(list);
+	int err;
+
+	err = ovl_rename_start(&ovlrd, &list);
+	if (err)
+		goto out;
+
+	old_cred = ovl_override_creds(old->d_sb);
+
+	err = ovl_rename_upper(&ovlrd, &list);
+
+	ovl_revert_creds(old_cred);
+	ovl_rename_end(&ovlrd);
 out:
 	dput(ovlrd.new_upper);
 	dput(ovlrd.old_upper);
 	dput(ovlrd.opaquedir);
 	ovl_cache_free(&list);
 	return err;
-
-out_unlock:
-	unlock_rename(new_upperdir, old_upperdir);
-	goto out_revert_creds;
 }
 
 static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
-- 
2.51.1


From 6bcfb42324149f68862ec9fc73f02108075221aa Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 14 Nov 2025 17:16:09 +0100
Subject: [PATCH 3/3] ovl: port ovl_rename() to cred guard

Use the scoped ovl cred guard.

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

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1baab90889167..e42a6da981c9e 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1321,7 +1321,6 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 		      struct dentry *old, struct inode *newdir,
 		      struct dentry *new, unsigned int flags)
 {
-	const struct cred *old_cred = NULL;
 	struct ovl_renamedata ovlrd = {
 		.old_parent		= old->d_parent,
 		.old_dentry		= old,
@@ -1337,11 +1336,9 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(old->d_sb);
+	with_ovl_creds(old->d_sb)
+		err = ovl_rename_upper(&ovlrd, &list);
 
-	err = ovl_rename_upper(&ovlrd, &list);
-
-	ovl_revert_creds(old_cred);
 	ovl_rename_end(&ovlrd);
 out:
 	dput(ovlrd.new_upper);
-- 
2.51.1