linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: linux-nfs@vger.kernel.org, kbuild test robot <fengguang.wu@intel.com>
Subject: [PATCH 1/2] NFS: fix two problems in lookup_revalidate in RCU-walk
Date: Mon, 04 Aug 2014 16:24:00 +1000	[thread overview]
Message-ID: <20140804062400.7621.10041.stgit@notabene.brown> (raw)
In-Reply-To: <20140804062225.7621.70050.stgit@notabene.brown>

1/ rcu_dereference isn't correct: that field isn't
   RCU protected.   It could potentially change at any time
   so ACCESS_ONCE might be justified.

   changes to ->d_parent are protected by ->d_seq.  However
   that isn't always checked after ->d_revalidate is called,
   so it is safest to keep the double-check that ->d_parent
   hasn't changed at the end of these functions.

2/ in nfs4_lookup_revalidate, "->d_parent" was forgotten.
   So 'parent' was not the parent of 'dentry'.
   This fails safe is the context is that dentry->d_inode is
   NULL, and the result of parent->d_inode being NULL is
   that ECHILD is returned, which is always safe.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/nfs/dir.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e754d205ea54..0295f78f2976 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1102,7 +1102,7 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 	int error;
 
 	if (flags & LOOKUP_RCU) {
-		parent = rcu_dereference(dentry->d_parent);
+		parent = ACCESS_ONCE(dentry->d_parent);
 		dir = ACCESS_ONCE(parent->d_inode);
 		if (!dir)
 			return -ECHILD;
@@ -1184,7 +1184,7 @@ out_set_verifier:
 	nfs_advise_use_readdirplus(dir);
  out_valid_noent:
 	if (flags & LOOKUP_RCU) {
-		if (parent != rcu_dereference(dentry->d_parent))
+		if (parent != ACCESS_ONCE(dentry->d_parent))
 			return -ECHILD;
 	} else
 		dput(parent);
@@ -1585,7 +1585,7 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 		struct inode *dir;
 
 		if (flags & LOOKUP_RCU) {
-			parent = rcu_dereference(dentry);
+			parent = ACCESS_ONCE(dentry->d_parent);
 			dir = ACCESS_ONCE(parent->d_inode);
 			if (!dir)
 				return -ECHILD;
@@ -1599,7 +1599,7 @@ static int nfs4_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 			ret = -ECHILD;
 		if (!(flags & LOOKUP_RCU))
 			dput(parent);
-		else if (parent != rcu_dereference(dentry))
+		else if (parent != ACCESS_ONCE(dentry->d_parent))
 			return -ECHILD;
 		goto out;
 	}



      parent reply	other threads:[~2014-08-04  6:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-04  6:24 [PATCH 0/2] Two fixes for NFS RCU-walk support NeilBrown
2014-08-04  6:24 ` [PATCH 2/2] SUNRPC: remove all refcounting of groupinfo from rpcauth_lookupcred NeilBrown
2014-08-04  6:24 ` NeilBrown [this message]

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=20140804062400.7621.10041.stgit@notabene.brown \
    --to=neilb@suse.de \
    --cc=fengguang.wu@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).