diff mbox series

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

Message ID 165451866750136@kroah.com (mailing list archive)
State New, archived
Headers show
Series FAILED: patch "[PATCH] exportfs: support idmapped mounts" failed to apply to 5.4-stable tree | expand

Commit Message

Greg Kroah-Hartman June 6, 2022, 12:31 p.m. UTC 
The patch below does not apply to the 5.4-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 <stable@vger.kernel.org>.

thanks,

greg k-h

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

From 3a761d72fa62eec8913e45d29375344f61706541 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
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: <stable@vger.kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>
Tested-by: Giuseppe Scrivano <gscrivan@redhat.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

Comments

kernel test robot June 7, 2022, 2:46 p.m. UTC | #1 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on hch-configfs/for-next]
[cannot apply to linus/master v5.19-rc1 next-20220607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/gregkh-linuxfoundation-org/FAILED-patch-PATCH-exportfs-support-idmapped-mounts-failed-to-apply-to-5-4-stable-tree/20220606-203330
base:   git://git.infradead.org/users/hch/configfs.git for-next
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220607/202206072252.SYRt38ih-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/cda5e21742a0ec193c2dfd7e445a2024f9685eb9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review gregkh-linuxfoundation-org/FAILED-patch-PATCH-exportfs-support-idmapped-mounts-failed-to-apply-to-5-4-stable-tree/20220606-203330
        git checkout cda5e21742a0ec193c2dfd7e445a2024f9685eb9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/exportfs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/exportfs/expfs.c: In function 'reconnect_one':
   fs/exportfs/expfs.c:148:15: error: implicit declaration of function 'lookup_one_unlocked'; did you mean 'lookup_one_len_unlocked'? [-Werror=implicit-function-declaration]
     148 |         tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
         |               ^~~~~~~~~~~~~~~~~~~
         |               lookup_one_len_unlocked
>> fs/exportfs/expfs.c:148:13: warning: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     148 |         tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
         |             ^
   cc1: some warnings being treated as errors


vim +148 fs/exportfs/expfs.c

   108	
   109	/*
   110	 * Reconnect a directory dentry with its parent.
   111	 *
   112	 * This can return a dentry, or NULL, or an error.
   113	 *
   114	 * In the first case the returned dentry is the parent of the given
   115	 * dentry, and may itself need to be reconnected to its parent.
   116	 *
   117	 * In the NULL case, a concurrent VFS operation has either renamed or
   118	 * removed this directory.  The concurrent operation has reconnected our
   119	 * dentry, so we no longer need to.
   120	 */
   121	static struct dentry *reconnect_one(struct vfsmount *mnt,
   122			struct dentry *dentry, char *nbuf)
   123	{
   124		struct dentry *parent;
   125		struct dentry *tmp;
   126		int err;
   127	
   128		parent = ERR_PTR(-EACCES);
   129		inode_lock(dentry->d_inode);
   130		if (mnt->mnt_sb->s_export_op->get_parent)
   131			parent = mnt->mnt_sb->s_export_op->get_parent(dentry);
   132		inode_unlock(dentry->d_inode);
   133	
   134		if (IS_ERR(parent)) {
   135			dprintk("%s: get_parent of %ld failed, err %d\n",
   136				__func__, dentry->d_inode->i_ino, PTR_ERR(parent));
   137			return parent;
   138		}
   139	
   140		dprintk("%s: find name of %lu in %lu\n", __func__,
   141			dentry->d_inode->i_ino, parent->d_inode->i_ino);
   142		err = exportfs_get_name(mnt, parent, nbuf, dentry);
   143		if (err == -ENOENT)
   144			goto out_reconnected;
   145		if (err)
   146			goto out_err;
   147		dprintk("%s: found name: %s\n", __func__, nbuf);
 > 148		tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
   149		if (IS_ERR(tmp)) {
   150			dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
   151			err = PTR_ERR(tmp);
   152			goto out_err;
   153		}
   154		if (tmp != dentry) {
   155			/*
   156			 * Somebody has renamed it since exportfs_get_name();
   157			 * great, since it could've only been renamed if it
   158			 * got looked up and thus connected, and it would
   159			 * remain connected afterwards.  We are done.
   160			 */
   161			dput(tmp);
   162			goto out_reconnected;
   163		}
   164		dput(tmp);
   165		if (IS_ROOT(dentry)) {
   166			err = -ESTALE;
   167			goto out_err;
   168		}
   169		return parent;
   170	
   171	out_err:
   172		dput(parent);
   173		return ERR_PTR(err);
   174	out_reconnected:
   175		dput(parent);
   176		/*
   177		 * Someone must have renamed our entry into another parent, in
   178		 * which case it has been reconnected by the rename.
   179		 *
   180		 * Or someone removed it entirely, in which case filehandle
   181		 * lookup will succeed but the directory is now IS_DEAD and
   182		 * subsequent operations on it will fail.
   183		 *
   184		 * Alternatively, maybe there was no race at all, and the
   185		 * filesystem is just corrupt and gave us a parent that doesn't
   186		 * actually contain any entry pointing to this inode.  So,
   187		 * double check that this worked and return -ESTALE if not:
   188		 */
   189		if (!dentry_connected(dentry))
   190			return ERR_PTR(-ESTALE);
   191		return NULL;
   192	}
   193
kernel test robot June 7, 2022, 5:07 p.m. UTC | #2 
Hi,

I love your patch! Yet something to improve:

[auto build test ERROR on hch-configfs/for-next]
[cannot apply to linus/master v5.19-rc1 next-20220607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/gregkh-linuxfoundation-org/FAILED-patch-PATCH-exportfs-support-idmapped-mounts-failed-to-apply-to-5-4-stable-tree/20220606-203330
base:   git://git.infradead.org/users/hch/configfs.git for-next
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220608/202206080035.k0B6pmUd-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/cda5e21742a0ec193c2dfd7e445a2024f9685eb9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review gregkh-linuxfoundation-org/FAILED-patch-PATCH-exportfs-support-idmapped-mounts-failed-to-apply-to-5-4-stable-tree/20220606-203330
        git checkout cda5e21742a0ec193c2dfd7e445a2024f9685eb9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/exportfs/expfs.c: In function 'reconnect_one':
>> fs/exportfs/expfs.c:148:15: error: implicit declaration of function 'lookup_one_unlocked'; did you mean 'lookup_one_len_unlocked'? [-Werror=implicit-function-declaration]
     148 |         tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
         |               ^~~~~~~~~~~~~~~~~~~
         |               lookup_one_len_unlocked
   fs/exportfs/expfs.c:148:13: warning: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     148 |         tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
         |             ^
   cc1: some warnings being treated as errors


vim +148 fs/exportfs/expfs.c

   108	
   109	/*
   110	 * Reconnect a directory dentry with its parent.
   111	 *
   112	 * This can return a dentry, or NULL, or an error.
   113	 *
   114	 * In the first case the returned dentry is the parent of the given
   115	 * dentry, and may itself need to be reconnected to its parent.
   116	 *
   117	 * In the NULL case, a concurrent VFS operation has either renamed or
   118	 * removed this directory.  The concurrent operation has reconnected our
   119	 * dentry, so we no longer need to.
   120	 */
   121	static struct dentry *reconnect_one(struct vfsmount *mnt,
   122			struct dentry *dentry, char *nbuf)
   123	{
   124		struct dentry *parent;
   125		struct dentry *tmp;
   126		int err;
   127	
   128		parent = ERR_PTR(-EACCES);
   129		inode_lock(dentry->d_inode);
   130		if (mnt->mnt_sb->s_export_op->get_parent)
   131			parent = mnt->mnt_sb->s_export_op->get_parent(dentry);
   132		inode_unlock(dentry->d_inode);
   133	
   134		if (IS_ERR(parent)) {
   135			dprintk("%s: get_parent of %ld failed, err %d\n",
   136				__func__, dentry->d_inode->i_ino, PTR_ERR(parent));
   137			return parent;
   138		}
   139	
   140		dprintk("%s: find name of %lu in %lu\n", __func__,
   141			dentry->d_inode->i_ino, parent->d_inode->i_ino);
   142		err = exportfs_get_name(mnt, parent, nbuf, dentry);
   143		if (err == -ENOENT)
   144			goto out_reconnected;
   145		if (err)
   146			goto out_err;
   147		dprintk("%s: found name: %s\n", __func__, nbuf);
 > 148		tmp = lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nbuf));
   149		if (IS_ERR(tmp)) {
   150			dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
   151			err = PTR_ERR(tmp);
   152			goto out_err;
   153		}
   154		if (tmp != dentry) {
   155			/*
   156			 * Somebody has renamed it since exportfs_get_name();
   157			 * great, since it could've only been renamed if it
   158			 * got looked up and thus connected, and it would
   159			 * remain connected afterwards.  We are done.
   160			 */
   161			dput(tmp);
   162			goto out_reconnected;
   163		}
   164		dput(tmp);
   165		if (IS_ROOT(dentry)) {
   166			err = -ESTALE;
   167			goto out_err;
   168		}
   169		return parent;
   170	
   171	out_err:
   172		dput(parent);
   173		return ERR_PTR(err);
   174	out_reconnected:
   175		dput(parent);
   176		/*
   177		 * Someone must have renamed our entry into another parent, in
   178		 * which case it has been reconnected by the rename.
   179		 *
   180		 * Or someone removed it entirely, in which case filehandle
   181		 * lookup will succeed but the directory is now IS_DEAD and
   182		 * subsequent operations on it will fail.
   183		 *
   184		 * Alternatively, maybe there was no race at all, and the
   185		 * filesystem is just corrupt and gave us a parent that doesn't
   186		 * actually contain any entry pointing to this inode.  So,
   187		 * double check that this worked and return -ESTALE if not:
   188		 */
   189		if (!dentry_connected(dentry))
   190			return ERR_PTR(-ESTALE);
   191		return NULL;
   192	}
   193
Christian Brauner June 7, 2022, 6:08 p.m. UTC | #3 
On Mon, Jun 06, 2022 at 02:31:07PM +0200, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 5.4-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 <stable@vger.kernel.org>.
> 
> thanks,
> 
> greg k-h

Just fyi, this doesn't need to be backported to anything prior to 5.12.

Christian
diff mbox series

Patch

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 0106eba46d5a..3ef80d000e13 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -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);