Revert "ovl: do not encode lower fh with upper sb_writers held"

This reverts commit 26423e18cd which is
commit 5b02bfc1e7 upstream.

It is reported to part of a series that causes problems in the 6.6.y
tree, so revert it at this point in time and it can come back later if
still needed.

Reported-by: Ignat Korchagin <ignat@cloudflare.com>
Link: https://lore.kernel.org/r/ACD4D6CC-C4D5-4657-A805-03C34559046E@cloudflare.com
Cc: Dmitry Safonov <dima@arista.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

fs/overlayfs/copy_up.c

@@ -426,29 +426,29 @@ out_err:
 	return ERR_PTR(err);
 }
 
-struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin)
+int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
+		   struct dentry *upper)
 {
+	const struct ovl_fh *fh = NULL;
+	int err;
+
 	/*
 	 * When lower layer doesn't support export operations store a 'null' fh,
 	 * so we can use the overlay.origin xattr to distignuish between a copy
 	 * up and a pure upper inode.
 	 */
-	if (!ovl_can_decode_fh(origin->d_sb))
-		return NULL;
-
-	return ovl_encode_real_fh(ofs, origin, false);
-}
-
-int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
-		      struct dentry *upper)
-{
-	int err;
+	if (ovl_can_decode_fh(lower->d_sb)) {
+		fh = ovl_encode_real_fh(ofs, lower, false);
+		if (IS_ERR(fh))
+			return PTR_ERR(fh);
+	}
 
 	/*
 	 * Do not fail when upper doesn't support xattrs.
 	 */
 	err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh->buf,
 				 fh ? fh->fb.len : 0, 0);
+	kfree(fh);
 
 	/* Ignore -EPERM from setting "user.*" on symlink/special */
 	return err == -EPERM ? 0 : err;
@@ -476,7 +476,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
  *
  * Caller must hold i_mutex on indexdir.
  */
-static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
+static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
 			    struct dentry *upper)
 {
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
@@ -502,7 +502,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
 	if (WARN_ON(ovl_test_flag(OVL_INDEX, d_inode(dentry))))
 		return -EIO;
 
-	err = ovl_get_index_name_fh(fh, &name);
+	err = ovl_get_index_name(ofs, origin, &name);
 	if (err)
 		return err;
 
@@ -541,7 +541,6 @@ struct ovl_copy_up_ctx {
 	struct dentry *destdir;
 	struct qstr destname;
 	struct dentry *workdir;
-	const struct ovl_fh *origin_fh;
 	bool origin;
 	bool indexed;
 	bool metacopy;
@@ -638,7 +637,7 @@ static int ovl_copy_up_metadata(struct ovl_copy_up_ctx *c, struct dentry *temp)
 	 * hard link.
 	 */
 	if (c->origin) {
-		err = ovl_set_origin_fh(ofs, c->origin_fh, temp);
+		err = ovl_set_origin(ofs, c->lowerpath.dentry, temp);
 		if (err)
 			return err;
 	}
@@ -750,7 +749,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
 		goto cleanup;
 
 	if (S_ISDIR(c->stat.mode) && c->indexed) {
-		err = ovl_create_index(c->dentry, c->origin_fh, temp);
+		err = ovl_create_index(c->dentry, c->lowerpath.dentry, temp);
 		if (err)
 			goto cleanup;
 	}
@@ -862,8 +861,6 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 {
 	int err;
 	struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
-	struct dentry *origin = c->lowerpath.dentry;
-	struct ovl_fh *fh = NULL;
 	bool to_index = false;
 
 	/*
@@ -880,25 +877,17 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 			to_index = true;
 	}
 
-	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index) {
-		fh = ovl_get_origin_fh(ofs, origin);
-		if (IS_ERR(fh))
-			return PTR_ERR(fh);
-
-		/* origin_fh may be NULL */
-		c->origin_fh = fh;
+	if (S_ISDIR(c->stat.mode) || c->stat.nlink == 1 || to_index)
 		c->origin = true;
-	}
 
 	if (to_index) {
 		c->destdir = ovl_indexdir(c->dentry->d_sb);
-		err = ovl_get_index_name(ofs, origin, &c->destname);
+		err = ovl_get_index_name(ofs, c->lowerpath.dentry, &c->destname);
 		if (err)
-			goto out_free_fh;
+			return err;
 	} else if (WARN_ON(!c->parent)) {
 		/* Disconnected dentry must be copied up to index dir */
-		err = -EIO;
-		goto out_free_fh;
+		return -EIO;
 	} else {
 		/*
 		 * Mark parent "impure" because it may now contain non-pure
@@ -906,7 +895,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 		 */
 		err = ovl_set_impure(c->parent, c->destdir);
 		if (err)
-			goto out_free_fh;
+			return err;
 	}
 
 	/* Should we copyup with O_TMPFILE or with workdir? */
@@ -938,8 +927,6 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
 out:
 	if (to_index)
 		kfree(c->destname.name);
-out_free_fh:
-	kfree(fh);
 	return err;
 }
 

fs/overlayfs/namei.c

@@ -507,19 +507,6 @@ static int ovl_verify_fh(struct ovl_fs *ofs, struct dentry *dentry,
 	return err;
 }
 
-int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
-		      enum ovl_xattr ox, const struct ovl_fh *fh,
-		      bool is_upper, bool set)
-{
-	int err;
-
-	err = ovl_verify_fh(ofs, dentry, ox, fh);
-	if (set && err == -ENODATA)
-		err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
-
-	return err;
-}
-
 /*
  * Verify that @real dentry matches the file handle stored in xattr @name.
  *
@@ -528,9 +515,9 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
  *
  * Return 0 on match, -ESTALE on mismatch, -ENODATA on no xattr, < 0 on error.
  */
-int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
-			    enum ovl_xattr ox, struct dentry *real,
-			    bool is_upper, bool set)
+int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
+		      enum ovl_xattr ox, struct dentry *real, bool is_upper,
+		      bool set)
 {
 	struct inode *inode;
 	struct ovl_fh *fh;
@@ -543,7 +530,9 @@ int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
 		goto fail;
 	}
 
-	err = ovl_verify_set_fh(ofs, dentry, ox, fh, is_upper, set);
+	err = ovl_verify_fh(ofs, dentry, ox, fh);
+	if (set && err == -ENODATA)
+		err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
 	if (err)
 		goto fail;
 
@@ -559,7 +548,6 @@ fail:
 	goto out;
 }
 
-
 /* Get upper dentry from index */
 struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index,
 			       bool connected)
@@ -696,7 +684,7 @@ orphan:
 	goto out;
 }
 
-int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name)
+static int ovl_get_index_name_fh(struct ovl_fh *fh, struct qstr *name)
 {
 	char *n, *s;
 
@@ -885,27 +873,20 @@ int ovl_path_next(int idx, struct dentry *dentry, struct path *path)
 static int ovl_fix_origin(struct ovl_fs *ofs, struct dentry *dentry,
 			  struct dentry *lower, struct dentry *upper)
 {
-	const struct ovl_fh *fh;
 	int err;
 
 	if (ovl_check_origin_xattr(ofs, upper))
 		return 0;
 
-	fh = ovl_get_origin_fh(ofs, lower);
-	if (IS_ERR(fh))
-		return PTR_ERR(fh);
-
 	err = ovl_want_write(dentry);
 	if (err)
-		goto out;
+		return err;
 
-	err = ovl_set_origin_fh(ofs, fh, upper);
+	err = ovl_set_origin(ofs, lower, upper);
 	if (!err)
 		err = ovl_set_impure(dentry->d_parent, upper->d_parent);
 
 	ovl_drop_write(dentry);
-out:
-	kfree(fh);
 	return err;
 }
 

fs/overlayfs/overlayfs.h

@@ -632,15 +632,11 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
 int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
 			struct dentry *upperdentry, struct ovl_path **stackp);
 int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,
-		      enum ovl_xattr ox, const struct ovl_fh *fh,
-		      bool is_upper, bool set);
-int ovl_verify_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry,
-			    enum ovl_xattr ox, struct dentry *real,
-			    bool is_upper, bool set);
+		      enum ovl_xattr ox, struct dentry *real, bool is_upper,
+		      bool set);
 struct dentry *ovl_index_upper(struct ovl_fs *ofs, struct dentry *index,
 			       bool connected);
 int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index);
-int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name);
 int ovl_get_index_name(struct ovl_fs *ofs, struct dentry *origin,
 		       struct qstr *name);
 struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh);
@@ -652,24 +648,17 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			  unsigned int flags);
 bool ovl_lower_positive(struct dentry *dentry);
 
-static inline int ovl_verify_origin_fh(struct ovl_fs *ofs, struct dentry *upper,
-				       const struct ovl_fh *fh, bool set)
-{
-	return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, fh, false, set);
-}
-
 static inline int ovl_verify_origin(struct ovl_fs *ofs, struct dentry *upper,
 				    struct dentry *origin, bool set)
 {
-	return ovl_verify_origin_xattr(ofs, upper, OVL_XATTR_ORIGIN, origin,
-				       false, set);
+	return ovl_verify_set_fh(ofs, upper, OVL_XATTR_ORIGIN, origin,
+				 false, set);
 }
 
 static inline int ovl_verify_upper(struct ovl_fs *ofs, struct dentry *index,
 				   struct dentry *upper, bool set)
 {
-	return ovl_verify_origin_xattr(ofs, index, OVL_XATTR_UPPER, upper,
-				       true, set);
+	return ovl_verify_set_fh(ofs, index, OVL_XATTR_UPPER, upper, true, set);
 }
 
 /* readdir.c */
@@ -834,9 +823,8 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *path, struct dentr
 int ovl_set_attr(struct ovl_fs *ofs, struct dentry *upper, struct kstat *stat);
 struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
 				  bool is_upper);
-struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin);
-int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
-		      struct dentry *upper);
+int ovl_set_origin(struct ovl_fs *ofs, struct dentry *lower,
+		   struct dentry *upper);
 
 /* export.c */
 extern const struct export_operations ovl_export_operations;

fs/overlayfs/super.c

@@ -879,20 +879,15 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
 {
 	struct vfsmount *mnt = ovl_upper_mnt(ofs);
 	struct dentry *indexdir;
-	struct dentry *origin = ovl_lowerstack(oe)->dentry;
-	const struct ovl_fh *fh;
 	int err;
 
-	fh = ovl_get_origin_fh(ofs, origin);
-	if (IS_ERR(fh))
-		return PTR_ERR(fh);
-
 	err = mnt_want_write(mnt);
 	if (err)
-		goto out_free_fh;
+		return err;
 
 	/* Verify lower root is upper root origin */
-	err = ovl_verify_origin_fh(ofs, upperpath->dentry, fh, true);
+	err = ovl_verify_origin(ofs, upperpath->dentry,
+				ovl_lowerstack(oe)->dentry, true);
 	if (err) {
 		pr_err("failed to verify upper root origin\n");
 		goto out;
@@ -924,10 +919,9 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
 		 * directory entries.
 		 */
 		if (ovl_check_origin_xattr(ofs, ofs->indexdir)) {
-			err = ovl_verify_origin_xattr(ofs, ofs->indexdir,
-						      OVL_XATTR_ORIGIN,
-						      upperpath->dentry, true,
-						      false);
+			err = ovl_verify_set_fh(ofs, ofs->indexdir,
+						OVL_XATTR_ORIGIN,
+						upperpath->dentry, true, false);
 			if (err)
 				pr_err("failed to verify index dir 'origin' xattr\n");
 		}
@@ -945,8 +939,6 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs,
 
 out:
 	mnt_drop_write(mnt);
-out_free_fh:
-	kfree(fh);
 	return err;
 }
 

fs/overlayfs/util.c

@@ -976,18 +976,12 @@ static void ovl_cleanup_index(struct dentry *dentry)
 	struct dentry *index = NULL;
 	struct inode *inode;
 	struct qstr name = { };
-	bool got_write = false;
 	int err;
 
 	err = ovl_get_index_name(ofs, lowerdentry, &name);
 	if (err)
 		goto fail;
 
-	err = ovl_want_write(dentry);
-	if (err)
-		goto fail;
-
-	got_write = true;
 	inode = d_inode(upperdentry);
 	if (!S_ISDIR(inode->i_mode) && inode->i_nlink != 1) {
 		pr_warn_ratelimited("cleanup linked index (%pd2, ino=%lu, nlink=%u)\n",
@@ -1025,8 +1019,6 @@ static void ovl_cleanup_index(struct dentry *dentry)
 		goto fail;
 
 out:
-	if (got_write)
-		ovl_drop_write(dentry);
 	kfree(name.name);
 	dput(index);
 	return;
@@ -1097,8 +1089,6 @@ void ovl_nlink_end(struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
 
-	ovl_drop_write(dentry);
-
 	if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) {
 		const struct cred *old_cred;