FAILED: patch "[PATCH] exportfs: support idmapped mounts" failed to apply to 5.4-stable tree
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
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);
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/FA... 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@i...) 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/cda5e21742a0ec193c2dfd7e445a20... 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
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/FA... 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@i...) 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/cda5e21742a0ec193c2dfd7e445a20... 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
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
linux-stable-mirror@lists.linaro.org
-
Christian Brauner -
gregkh＠linuxfoundation.org -
kernel test robot