aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Woodhouse <dwmw2@infradead.org>2009-04-20 18:18:37 -0400
committerAl Viro <viro@zeniv.linux.org.uk>2009-04-20 23:01:16 -0400
commit2f9092e1020246168b1309b35e085ecd7ff9ff72 (patch)
treef8318c1e62e789718ae7637869f6c075b815bcb2
parent1ba0c7dbbbc24230394100c5f0d0df38cb400cff (diff)
Fix i_mutex vs. readdir handling in nfsd
Commit 14f7dd63 ("Copy XFS readdir hack into nfsd code") introduced a bug to generic code which had been extant for a long time in the XFS version -- it started to call through into lookup_one_len() and hence into the file systems' ->lookup() methods without i_mutex held on the directory. This patch fixes it by locking the directory's i_mutex again before calling the filldir functions. The original deadlocks which commit 14f7dd63 was designed to avoid are still avoided, because they were due to fs-internal locking, not i_mutex. While we're at it, fix the return type of nfsd_buffered_readdir() which should be a __be32 not an int -- it's an NFS errno, not a Linux errno. And return nfserrno(-ENOMEM) when allocation fails, not just -ENOMEM. Sparse would have caught that, if it wasn't so busy bitching about __cold__. Commit 05f4f678 ("nfsd4: don't do lookup within readdir in recovery code") introduced a similar problem with calling lookup_one_len() without i_mutex, which this patch also addresses. To fix that, it was necessary to fix the called functions so that they expect i_mutex to be held; that part was done by J. Bruce Fields. Signed-off-by: David Woodhouse <David.Woodhouse@intel.com> Umm-I-can-live-with-that-by: Al Viro <viro@zeniv.linux.org.uk> Reported-by: J. R. Okajima <hooanon05@yahoo.co.jp> Tested-by: J. Bruce Fields <bfields@citi.umich.edu> LKML-Reference: <8036.1237474444@jrobl> Cc: stable@kernel.org Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-rw-r--r--fs/namei.c2
-rw-r--r--fs/nfsd/nfs4recover.c46
-rw-r--r--fs/nfsd/vfs.c25
3 files changed, 30 insertions, 43 deletions
diff --git a/fs/namei.c b/fs/namei.c
index b8433ebfae05..78f253cd2d4f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1248,6 +1248,8 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
1248 int err; 1248 int err;
1249 struct qstr this; 1249 struct qstr this;
1250 1250
1251 WARN_ON_ONCE(!mutex_is_locked(&base->d_inode->i_mutex));
1252
1251 err = __lookup_one_len(name, &this, base, len); 1253 err = __lookup_one_len(name, &this, base, len);
1252 if (err) 1254 if (err)
1253 return ERR_PTR(err); 1255 return ERR_PTR(err);
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 3444c0052a87..5275097a7565 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -229,21 +229,23 @@ nfsd4_list_rec_dir(struct dentry *dir, recdir_func *f)
229 goto out; 229 goto out;
230 status = vfs_readdir(filp, nfsd4_build_namelist, &names); 230 status = vfs_readdir(filp, nfsd4_build_namelist, &names);
231 fput(filp); 231 fput(filp);
232 mutex_lock(&dir->d_inode->i_mutex);
232 while (!list_empty(&names)) { 233 while (!list_empty(&names)) {
233 entry = list_entry(names.next, struct name_list, list); 234 entry = list_entry(names.next, struct name_list, list);
234 235
235 dentry = lookup_one_len(entry->name, dir, HEXDIR_LEN-1); 236 dentry = lookup_one_len(entry->name, dir, HEXDIR_LEN-1);
236 if (IS_ERR(dentry)) { 237 if (IS_ERR(dentry)) {
237 status = PTR_ERR(dentry); 238 status = PTR_ERR(dentry);
238 goto out; 239 break;
239 } 240 }
240 status = f(dir, dentry); 241 status = f(dir, dentry);
241 dput(dentry); 242 dput(dentry);
242 if (status) 243 if (status)
243 goto out; 244 break;
244 list_del(&entry->list); 245 list_del(&entry->list);
245 kfree(entry); 246 kfree(entry);
246 } 247 }
248 mutex_unlock(&dir->d_inode->i_mutex);
247out: 249out:
248 while (!list_empty(&names)) { 250 while (!list_empty(&names)) {
249 entry = list_entry(names.next, struct name_list, list); 251 entry = list_entry(names.next, struct name_list, list);
@@ -255,36 +257,6 @@ out:
255} 257}
256 258
257static int 259static int
258nfsd4_remove_clid_file(struct dentry *dir, struct dentry *dentry)
259{
260 int status;
261
262 if (!S_ISREG(dir->d_inode->i_mode)) {
263 printk("nfsd4: non-file found in client recovery directory\n");
264 return -EINVAL;
265 }
266 mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
267 status = vfs_unlink(dir->d_inode, dentry);
268 mutex_unlock(&dir->d_inode->i_mutex);
269 return status;
270}
271
272static int
273nfsd4_clear_clid_dir(struct dentry *dir, struct dentry *dentry)
274{
275 int status;
276
277 /* For now this directory should already be empty, but we empty it of
278 * any regular files anyway, just in case the directory was created by
279 * a kernel from the future.... */
280 nfsd4_list_rec_dir(dentry, nfsd4_remove_clid_file);
281 mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
282 status = vfs_rmdir(dir->d_inode, dentry);
283 mutex_unlock(&dir->d_inode->i_mutex);
284 return status;
285}
286
287static int
288nfsd4_unlink_clid_dir(char *name, int namlen) 260nfsd4_unlink_clid_dir(char *name, int namlen)
289{ 261{
290 struct dentry *dentry; 262 struct dentry *dentry;
@@ -294,18 +266,18 @@ nfsd4_unlink_clid_dir(char *name, int namlen)
294 266
295 mutex_lock(&rec_dir.dentry->d_inode->i_mutex); 267 mutex_lock(&rec_dir.dentry->d_inode->i_mutex);
296 dentry = lookup_one_len(name, rec_dir.dentry, namlen); 268 dentry = lookup_one_len(name, rec_dir.dentry, namlen);
297 mutex_unlock(&rec_dir.dentry->d_inode->i_mutex);
298 if (IS_ERR(dentry)) { 269 if (IS_ERR(dentry)) {
299 status = PTR_ERR(dentry); 270 status = PTR_ERR(dentry);
300 return status; 271 goto out_unlock;
301 } 272 }
302 status = -ENOENT; 273 status = -ENOENT;
303 if (!dentry->d_inode) 274 if (!dentry->d_inode)
304 goto out; 275 goto out;
305 276 status = vfs_rmdir(rec_dir.dentry->d_inode, dentry);
306 status = nfsd4_clear_clid_dir(rec_dir.dentry, dentry);
307out: 277out:
308 dput(dentry); 278 dput(dentry);
279out_unlock:
280 mutex_unlock(&rec_dir.dentry->d_inode->i_mutex);
309 return status; 281 return status;
310} 282}
311 283
@@ -348,7 +320,7 @@ purge_old(struct dentry *parent, struct dentry *child)
348 if (nfs4_has_reclaimed_state(child->d_name.name, false)) 320 if (nfs4_has_reclaimed_state(child->d_name.name, false))
349 return 0; 321 return 0;
350 322
351 status = nfsd4_clear_clid_dir(parent, child); 323 status = vfs_rmdir(parent->d_inode, child);
352 if (status) 324 if (status)
353 printk("failed to remove client recovery directory %s\n", 325 printk("failed to remove client recovery directory %s\n",
354 child->d_name.name); 326 child->d_name.name);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 46e6bd2d4f07..6c68ffd6b4bb 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1890,8 +1890,8 @@ static int nfsd_buffered_filldir(void *__buf, const char *name, int namlen,
1890 return 0; 1890 return 0;
1891} 1891}
1892 1892
1893static int nfsd_buffered_readdir(struct file *file, filldir_t func, 1893static __be32 nfsd_buffered_readdir(struct file *file, filldir_t func,
1894 struct readdir_cd *cdp, loff_t *offsetp) 1894 struct readdir_cd *cdp, loff_t *offsetp)
1895{ 1895{
1896 struct readdir_data buf; 1896 struct readdir_data buf;
1897 struct buffered_dirent *de; 1897 struct buffered_dirent *de;
@@ -1901,11 +1901,12 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,
1901 1901
1902 buf.dirent = (void *)__get_free_page(GFP_KERNEL); 1902 buf.dirent = (void *)__get_free_page(GFP_KERNEL);
1903 if (!buf.dirent) 1903 if (!buf.dirent)
1904 return -ENOMEM; 1904 return nfserrno(-ENOMEM);
1905 1905
1906 offset = *offsetp; 1906 offset = *offsetp;
1907 1907
1908 while (1) { 1908 while (1) {
1909 struct inode *dir_inode = file->f_path.dentry->d_inode;
1909 unsigned int reclen; 1910 unsigned int reclen;
1910 1911
1911 cdp->err = nfserr_eof; /* will be cleared on successful read */ 1912 cdp->err = nfserr_eof; /* will be cleared on successful read */
@@ -1924,26 +1925,38 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,
1924 if (!size) 1925 if (!size)
1925 break; 1926 break;
1926 1927
1928 /*
1929 * Various filldir functions may end up calling back into
1930 * lookup_one_len() and the file system's ->lookup() method.
1931 * These expect i_mutex to be held, as it would within readdir.
1932 */
1933 host_err = mutex_lock_killable(&dir_inode->i_mutex);
1934 if (host_err)
1935 break;
1936
1927 de = (struct buffered_dirent *)buf.dirent; 1937 de = (struct buffered_dirent *)buf.dirent;
1928 while (size > 0) { 1938 while (size > 0) {
1929 offset = de->offset; 1939 offset = de->offset;
1930 1940
1931 if (func(cdp, de->name, de->namlen, de->offset, 1941 if (func(cdp, de->name, de->namlen, de->offset,
1932 de->ino, de->d_type)) 1942 de->ino, de->d_type))
1933 goto done; 1943 break;
1934 1944
1935 if (cdp->err != nfs_ok) 1945 if (cdp->err != nfs_ok)
1936 goto done; 1946 break;
1937 1947
1938 reclen = ALIGN(sizeof(*de) + de->namlen, 1948 reclen = ALIGN(sizeof(*de) + de->namlen,
1939 sizeof(u64)); 1949 sizeof(u64));
1940 size -= reclen; 1950 size -= reclen;
1941 de = (struct buffered_dirent *)((char *)de + reclen); 1951 de = (struct buffered_dirent *)((char *)de + reclen);
1942 } 1952 }
1953 mutex_unlock(&dir_inode->i_mutex);
1954 if (size > 0) /* We bailed out early */
1955 break;
1956
1943 offset = vfs_llseek(file, 0, SEEK_CUR); 1957 offset = vfs_llseek(file, 0, SEEK_CUR);
1944 } 1958 }
1945 1959
1946 done:
1947 free_page((unsigned long)(buf.dirent)); 1960 free_page((unsigned long)(buf.dirent));
1948 1961
1949 if (host_err) 1962 if (host_err)