All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Coddington <bcodding@redhat.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>,
	Anna Schumaker <anna.schumaker@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH] NFSv4: Fix a dentry leak on alias use
Date: Wed, 17 Feb 2016 09:44:48 -0500	[thread overview]
Message-ID: <a65c14e6e3e051fd2367732ba4b237d3605920a6.1455718518.git.bcodding@redhat.com> (raw)

We've had some users hitting what used to be a BUG() in
shink_dcache_for_umount() but is now a printk:

BUG: Dentry ffff880066676000{i=20007,n=foobar} still in use (1) [unmount of nfs4 0:39]
VFS: Busy inodes after unmount of 0:39. Self-destruct in 5 seconds.  Have a nice day...

The users don't like that very much.

I've spotted a dentry leak in the rare case where d_add_unique() finds an
alias on open and we swap the open context's dentry.  I'm pretty sure we
shouldn't be doing another dget() there.  I can reliably reproduce it with
this bit of bash:

mkdir -p /exports/dir{1,2}
exportfs -o rw localhost:/exports
mount -t nfs -ov4.1 localhost:/ /mnt/localhost

(sleep 2) > /mnt/localhost/dir1/foobar &
waitpid="$!"
sleep 1
mv /exports/dir{1,2}/foobar
echo A > /mnt/localhost/dir2/foobar
stat /mnt/localhost/dir1/foobar 2> /dev/null
mv /exports/dir{2,1}/foobar
echo A > /mnt/localhost/dir1/foobar

wait $waitpid
umount /mnt/localhost

Ben

8<---------------------------------------------------------------------
In the case where d_add_unique() finds an appropriate alias to use it will
have already incremented the reference count.  An additional dget() to swap
the open context's dentry is unnecessary and will leak a reference.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/nfs4proc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 4bfc33a..f4401ed 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2468,7 +2468,7 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
 			dentry = opendata->dentry;
 		} else if (dentry != ctx->dentry) {
 			dput(ctx->dentry);
-			ctx->dentry = dget(dentry);
+			ctx->dentry = dentry;
 		}
 		nfs_set_verifier(dentry,
 				nfs_save_change_attribute(d_inode(opendata->dir)));
-- 
1.7.1


             reply	other threads:[~2016-02-17 14:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17 14:44 Benjamin Coddington [this message]
2016-02-17 15:41 ` [PATCH v2] NFSv4: Fix a dentry leak on alias use Benjamin Coddington

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a65c14e6e3e051fd2367732ba4b237d3605920a6.1455718518.git.bcodding@redhat.com \
    --to=bcodding@redhat.com \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.