[lkml]   [2009]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[PATCH] Fix i_mutex handling in nfsd readdir.
    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

    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 both of those if it wasn't so busy bitching
    about __cold__.

    Reported-by: J. R. Okajima <>
    Signed-off-by: David Woodhouse <>

    Commit 05f4f678b introduced a similar problem for NFSv4 which I'm not
    going to attempt to deal with before getting some sleep...

    diff --git a/fs/namei.c b/fs/namei.c
    index b8433eb..78f253c 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)
    int err;
    struct qstr this;

    + WARN_ON_ONCE(!mutex_is_locked(&base->d_inode->i_mutex));
    err = __lookup_one_len(name, &this, base, len);
    if (err)
    return ERR_PTR(err);
    diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
    index ab93fcf..44a68e1 100644
    --- a/fs/nfsd/vfs.c
    +++ b/fs/nfsd/vfs.c
    @@ -1885,8 +1885,8 @@ static int nfsd_buffered_filldir(void *__buf, const char *name, int namlen,
    return 0;

    -static int nfsd_buffered_readdir(struct file *file, filldir_t func,
    - struct readdir_cd *cdp, loff_t *offsetp)
    +static __be32 nfsd_buffered_readdir(struct file *file, filldir_t func,
    + struct readdir_cd *cdp, loff_t *offsetp)
    struct readdir_data buf;
    struct buffered_dirent *de;
    @@ -1896,11 +1896,12 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,

    buf.dirent = (void *)__get_free_page(GFP_KERNEL);
    if (!buf.dirent)
    - return -ENOMEM;
    + return nfserrno(-ENOMEM);

    offset = *offsetp;

    while (1) {
    + struct inode *dir_inode = file->f_path.dentry->d_inode;
    unsigned int reclen;

    cdp->err = nfserr_eof; /* will be cleared on successful read */
    @@ -1919,22 +1920,35 @@ static int nfsd_buffered_readdir(struct file *file, filldir_t func,
    if (!size)

    + /*
    + * Various filldir functions may end up calling back into
    + * lookup_one_len() and the file system's ->lookup() method.
    + * These expect i_mutex to be held, as it would within readdir.
    + */
    + host_err = mutex_lock_killable(&dir_inode->i_mutex);
    + if (host_err)
    + break;
    de = (struct buffered_dirent *)buf.dirent;
    while (size > 0) {
    offset = de->offset;

    if (func(cdp, de->name, de->namlen, de->offset,
    de->ino, de->d_type))
    - goto done;
    + break;

    if (cdp->err != nfs_ok)
    - goto done;
    + break;

    reclen = ALIGN(sizeof(*de) + de->namlen,
    size -= reclen;
    de = (struct buffered_dirent *)((char *)de + reclen);
    + mutex_unlock(&dir_inode->i_mutex);
    + if (size > 0) /* We bailed out early */
    + break;
    offset = vfs_llseek(file, 0, SEEK_CUR);

    David Woodhouse Open Source Technology Centre Intel Corporation

     \ /
      Last update: 2009-04-18 02:19    [W:0.027 / U:0.856 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site