FAILED: patch "[PATCH] exportfs: support idmapped mounts" failed to apply to 5.15-stable tree

PATCH In review [email protected] 2 participants 4 messages Jun 6, 2022 – Jun 7, 2022

Subsystems (3)

Stable branch VFS Filesystems exportfs

lore

Share & tools

lore

[email protected]

author maintainer

2022-06-06 12:31 UTC #0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 3a761d72fa62eec8913e45d29375344f61706541 Mon Sep 17 00:00:00 2001
From: Christian Brauner <[email protected]>
Date: Mon, 4 Apr 2022 12:51:41 +0200
Subject: [PATCH] exportfs: support idmapped mounts

Make the two locations where exportfs helpers check permission to lookup a given inode idmapped mount aware by switching it to the lookup_one() helper. This is a bugfix for the open_by_handle_at() system call which doesn't take idmapped mounts into account currently. It's not tied to a specific commit so we'll just Cc stable.

In addition this is required to support idmapped base layers in overlay. The overlay filesystem uses exportfs to encode and decode file handles for its index=on mount option and when nfs_export=on.

Cc <[email protected]>

Tested-by Giuseppe Scrivano <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner (Microsoft) <[email protected]>

Signed-off-by Miklos Szeredi <[email protected]>

fs/exportfs/expfs.c +3 -2

index 0106eba46d5a..3ef80d000e13 100644
@@ -145,7 +145,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
 	if (err)
 		goto out_err;
 	dprintk("%s: found name: %s\n", __func__, nbuf);
-	tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf));
+	tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
 	if (IS_ERR(tmp)) {
 		dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
 		err = PTR_ERR(tmp);

@@ -525,7 +525,8 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
 		}
 
 		inode_lock(target_dir->d_inode);
-		nresult = lookup_one_len(nbuf, target_dir, strlen(nbuf));
+		nresult = lookup_one(mnt_user_ns(mnt), nbuf,
+				     target_dir, strlen(nbuf));
 		if (!IS_ERR(nresult)) {
 			if (unlikely(nresult->d_inode != result->d_inode)) {
 				dput(nresult);

Christian Brauner

maintainer

2022-06-07 10:09 UTC #1 lore raw

PATCH 1/2 fs: add two trivial lookup helpers

Similar to the addition of lookup_one() add a version of lookup_one_unlocked() and lookup_one_positive_unlocked() that take idmapped mounts into account. This is required to port overlay to support idmapped base layers.

Cc <[email protected]>

Tested-by Giuseppe Scrivano <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner (Microsoft) <[email protected]>

Signed-off-by Miklos Szeredi <[email protected]>

Signed-off-by Christian Brauner (Microsoft) <[email protected]>

---
 fs/namei.c            | 70 ++++++++++++++++++++++++++++++++++++-------
 include/linux/namei.h |  6 ++++
 2 files changed, 66 insertions(+), 10 deletions(-)

fs/namei.c +60 ~~-10~~

index 8882a70dc119..2ea15d043412 100644
@@ -2718,7 +2718,8 @@ struct dentry *lookup_one(struct user_namespace *mnt_userns, const char *name,
 EXPORT_SYMBOL(lookup_one);
 
 /**
- * lookup_one_len_unlocked - filesystem helper to lookup single pathname component
+ * lookup_one_unlocked - filesystem helper to lookup single pathname component
+ * @mnt_userns:	idmapping of the mount the lookup is performed from
  * @name:	pathname component to lookup
  * @base:	base directory to lookup from
  * @len:	maximum length @len should be interpreted to

@@ -2729,14 +2730,15 @@ EXPORT_SYMBOL(lookup_one);
  * Unlike lookup_one_len, it should be called without the parent
  * i_mutex held, and will take the i_mutex itself if necessary.
  */
-struct dentry *lookup_one_len_unlocked(const char *name,
-				       struct dentry *base, int len)
+struct dentry *lookup_one_unlocked(struct user_namespace *mnt_userns,
+				   const char *name, struct dentry *base,
+				   int len)
 {
 	struct qstr this;
 	int err;
 	struct dentry *ret;
 
-	err = lookup_one_common(&init_user_ns, name, base, len, &this);
+	err = lookup_one_common(mnt_userns, name, base, len, &this);
 	if (err)
 		return ERR_PTR(err);

@@ -2745,6 +2747,59 @@ struct dentry *lookup_one_len_unlocked(const char *name,
 		ret = lookup_slow(&this, base, 0);
 	return ret;
 }
+EXPORT_SYMBOL(lookup_one_unlocked);
+
+/**
+ * lookup_one_positive_unlocked - filesystem helper to lookup single
+ *				  pathname component
+ * @mnt_userns:	idmapping of the mount the lookup is performed from
+ * @name:	pathname component to lookup
+ * @base:	base directory to lookup from
+ * @len:	maximum length @len should be interpreted to
+ *
+ * This helper will yield ERR_PTR(-ENOENT) on negatives. The helper returns
+ * known positive or ERR_PTR(). This is what most of the users want.
+ *
+ * Note that pinned negative with unlocked parent _can_ become positive at any
+ * time, so callers of lookup_one_unlocked() need to be very careful; pinned
+ * positives have >d_inode stable, so this one avoids such problems.
+ *
+ * Note that this routine is purely a helper for filesystem usage and should
+ * not be called by generic code.
+ *
+ * The helper should be called without i_mutex held.
+ */
+struct dentry *lookup_one_positive_unlocked(struct user_namespace *mnt_userns,
+					    const char *name,
+					    struct dentry *base, int len)
+{
+	struct dentry *ret = lookup_one_unlocked(mnt_userns, name, base, len);
+
+	if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) {
+		dput(ret);
+		ret = ERR_PTR(-ENOENT);
+	}
+	return ret;
+}
+EXPORT_SYMBOL(lookup_one_positive_unlocked);
+
+/**
+ * lookup_one_len_unlocked - filesystem helper to lookup single pathname component
+ * @name:	pathname component to lookup
+ * @base:	base directory to lookup from
+ * @len:	maximum length @len should be interpreted to
+ *
+ * Note that this routine is purely a helper for filesystem usage and should
+ * not be called by generic code.
+ *
+ * Unlike lookup_one_len, it should be called without the parent
+ * i_mutex held, and will take the i_mutex itself if necessary.
+ */
+struct dentry *lookup_one_len_unlocked(const char *name,
+				       struct dentry *base, int len)
+{
+	return lookup_one_unlocked(&init_user_ns, name, base, len);
+}
 EXPORT_SYMBOL(lookup_one_len_unlocked);
 
 /*

@@ -2758,12 +2813,7 @@ EXPORT_SYMBOL(lookup_one_len_unlocked);
 struct dentry *lookup_positive_unlocked(const char *name,
 				       struct dentry *base, int len)
 {
-	struct dentry *ret = lookup_one_len_unlocked(name, base, len);
-	if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) {
-		dput(ret);
-		ret = ERR_PTR(-ENOENT);
-	}
-	return ret;
+	return lookup_one_positive_unlocked(&init_user_ns, name, base, len);
 }
 EXPORT_SYMBOL(lookup_positive_unlocked);

include/linux/namei.h +6 -0

index e89329bb3134..caeb08a98536 100644
@@ -69,6 +69,12 @@ extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
 extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);
 extern struct dentry *lookup_positive_unlocked(const char *, struct dentry *, int);
 struct dentry *lookup_one(struct user_namespace *, const char *, struct dentry *, int);
+struct dentry *lookup_one_unlocked(struct user_namespace *mnt_userns,
+				   const char *name, struct dentry *base,
+				   int len);
+struct dentry *lookup_one_positive_unlocked(struct user_namespace *mnt_userns,
+					    const char *name,
+					    struct dentry *base, int len);
 
 extern int follow_down_one(struct path *);
 extern int follow_down(struct path *);

base-commit: 207ca688162d4d77129981a8b4352114b97a52b5

Signature · 1 line

2.34.1

Christian Brauner

maintainer

2022-06-07 10:09 UTC #2 lore raw

PATCH 2/2 exportfs: support idmapped mounts

In reply to [email protected]

In addition this is required to support idmapped base layers in overlay. The overlay filesystem uses exportfs to encode and decode file handles for its index=on mount option and when nfs_export=on.

Cc <[email protected]>

Tested-by Giuseppe Scrivano <[email protected]>

Reviewed-by Amir Goldstein <[email protected]>

Reviewed-by Christoph Hellwig <[email protected]>

Signed-off-by Christian Brauner (Microsoft) <[email protected]>

Signed-off-by Miklos Szeredi <[email protected]>

Signed-off-by Christian Brauner (Microsoft) <[email protected]>

---
Hey Greg,

This was missing a preliminary commit.
My build machines are currently down so I couldn't do a test build but this should build cleanly.

Thanks!
Christian

---
 fs/exportfs/expfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

fs/exportfs/expfs.c +3 -2

index 0106eba46d5a..3ef80d000e13 100644
@@ -145,7 +145,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
 	if (err)
 		goto out_err;
 	dprintk("%s: found name: %s\n", __func__, nbuf);
-	tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf));
+	tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
 	if (IS_ERR(tmp)) {
 		dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
 		err = PTR_ERR(tmp);

@@ -525,7 +525,8 @@ exportfs_decode_fh_raw(struct vfsmount *mnt, struct fid *fid, int fh_len,
 		}
 
 		inode_lock(target_dir->d_inode);
-		nresult = lookup_one_len(nbuf, target_dir, strlen(nbuf));
+		nresult = lookup_one(mnt_user_ns(mnt), nbuf,
+				     target_dir, strlen(nbuf));
 		if (!IS_ERR(nresult)) {
 			if (unlikely(nresult->d_inode != result->d_inode)) {
 				dput(nresult);

Signature · 1 line

2.34.1

Greg KH

author maintainer

2022-06-07 10:42 UTC #3 lore raw

Re: [PATCH 5.15.y 2/2] exportfs: support idmapped mounts

Quoted 18 lines · Christian Brauner

Make the two locations where exportfs helpers check permission to lookup a given inode idmapped mount aware by switching it to the lookup_one() helper. This is a bugfix for the open_by_handle_at() system call which doesn't take idmapped mounts into account currently. It's not tied to a specific commit so we'll just Cc stable.
In addition this is required to support idmapped base layers in overlay. The overlay filesystem uses exportfs to encode and decode file handles for its index=on mount option and when nfs_export=on.
Cc <[email protected]>
Cc <[email protected]>
Tested-by Giuseppe Scrivano <[email protected]>
Reviewed-by Amir Goldstein <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Christian Brauner (Microsoft) <[email protected]>
Signed-off-by Miklos Szeredi <[email protected]>
Signed-off-by Christian Brauner (Microsoft) <[email protected]>

Quoted 6 lines

---
Hey Greg,
This was missing a preliminary commit.
My build machines are currently down so I couldn't do a test build but this should build cleanly.

Looks like it applies cleanly, let me apply it and run it through my build systems...

thanks,

greg k-h