Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@primarydata.com>
To: bfields@fieldses.org
Cc: hch@infradead.org, linux-nfs@vger.kernel.org
Subject: [PATCH 09/11] nfsd: always hold the fi_lock when bumping fi_access refcounts
Date: Thu, 10 Jul 2014 14:07:33 -0400	[thread overview]
Message-ID: <1405015655-12469-10-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1405015655-12469-1-git-send-email-jlayton@primarydata.com>

Once we remove the client_mutex, there's an unlikely but possible race
that could occur. It will be possible for nfs4_file_put_access to race
with nfs4_file_get_access. The refcount will go to zero (briefly) and
then bumped back to one. If that happens we set ourselves up for a
use-after-free and the potential for a lock to race onto the i_flock
list as a filp is being torn down.

Ensure that we can safely bump the refcount on the file by holding the
fi_lock whenever that's done. The only place it currently isn't is in
get_lock_access.

In order to ensure atomicity with finding the file, use the
find_*_file_locked variants and then call get_lock_access to get new
access references on the nfs4_file under the same lock.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/nfsd/nfs4state.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5f7294712ad4..8f320f2f8b84 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -377,6 +377,8 @@ static struct hlist_head file_hashtbl[FILE_HASH_SIZE];
 static void
 __nfs4_file_get_access(struct nfs4_file *fp, u32 access)
 {
+	lockdep_assert_held(&fp->fi_lock);
+
 	if (access & NFS4_SHARE_ACCESS_WRITE)
 		atomic_inc(&fp->fi_access[O_WRONLY]);
 	if (access & NFS4_SHARE_ACCESS_READ)
@@ -386,6 +388,8 @@ __nfs4_file_get_access(struct nfs4_file *fp, u32 access)
 static __be32
 nfs4_file_get_access(struct nfs4_file *fp, u32 access)
 {
+	lockdep_assert_held(&fp->fi_lock);
+
 	/* Does this access mode make sense? */
 	if (access & ~NFS4_SHARE_ACCESS_BOTH)
 		return nfserr_inval;
@@ -4572,6 +4576,8 @@ static void get_lock_access(struct nfs4_ol_stateid *lock_stp, u32 access)
 {
 	struct nfs4_file *fp = lock_stp->st_file;
 
+	lockdep_assert_held(&fp->fi_lock);
+
 	if (test_access(access, lock_stp))
 		return;
 	__nfs4_file_get_access(fp, access);
@@ -4623,6 +4629,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_openowner *open_sop = NULL;
 	struct nfs4_lockowner *lock_sop = NULL;
 	struct nfs4_ol_stateid *lock_stp;
+	struct nfs4_file *fp;
 	struct file *filp = NULL;
 	struct file_lock *file_lock = NULL;
 	struct file_lock *conflock = NULL;
@@ -4703,20 +4710,25 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		goto out;
 	}
 
+	fp = lock_stp->st_file;
 	locks_init_lock(file_lock);
 	switch (lock->lk_type) {
 		case NFS4_READ_LT:
 		case NFS4_READW_LT:
-			filp = find_readable_file(lock_stp->st_file);
+			spin_lock(&fp->fi_lock);
+			filp = find_readable_file_locked(fp);
 			if (filp)
 				get_lock_access(lock_stp, NFS4_SHARE_ACCESS_READ);
+			spin_unlock(&fp->fi_lock);
 			file_lock->fl_type = F_RDLCK;
 			break;
 		case NFS4_WRITE_LT:
 		case NFS4_WRITEW_LT:
-			filp = find_writeable_file(lock_stp->st_file);
+			spin_lock(&fp->fi_lock);
+			filp = find_writeable_file_locked(fp);
 			if (filp)
 				get_lock_access(lock_stp, NFS4_SHARE_ACCESS_WRITE);
+			spin_unlock(&fp->fi_lock);
 			file_lock->fl_type = F_WRLCK;
 			break;
 		default:
-- 
1.9.3


  parent reply	other threads:[~2014-07-10 18:07 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-10 18:07 [PATCH 00/11] nfsd: deny mode handling overhaul Jeff Layton
2014-07-10 18:07 ` [PATCH 01/11] nfsd: Add fine grained protection for the nfs4_file->fi_stateids list Jeff Layton
2014-07-10 18:07 ` [PATCH 02/11] nfsd: Add locking to the nfs4_file->fi_fds[] array Jeff Layton
2014-07-10 18:07 ` [PATCH 03/11] nfsd: clean up helper __release_lock_stateid Jeff Layton
2014-07-10 18:07 ` [PATCH 04/11] nfsd: refactor nfs4_file_get_access and nfs4_file_put_access Jeff Layton
2014-07-10 18:07 ` [PATCH 05/11] nfsd: remove nfs4_file_put_fd Jeff Layton
2014-07-10 18:07 ` [PATCH 06/11] nfsd: shrink st_access_bmap and st_deny_bmap Jeff Layton
2014-07-10 18:07 ` [PATCH 07/11] nfsd: set stateid access and deny bits in nfs4_get_vfs_file Jeff Layton
2014-07-10 18:07 ` [PATCH 08/11] nfsd: clean up reset_union_bmap_deny Jeff Layton
2014-07-10 18:07 ` Jeff Layton [this message]
2014-07-10 18:07 ` [PATCH 10/11] nfsd: make deny mode enforcement more efficient and close races in it Jeff Layton
2014-07-10 20:08   ` J. Bruce Fields
2014-07-11 17:31   ` Frank Filz
2014-07-11 17:48     ` Jeff Layton
2014-07-11 17:56       ` Frank Filz
2014-07-11 18:00         ` Trond Myklebust
2014-07-11 18:07           ` Jeff Layton
2014-07-11 18:08           ` Frank Filz
2014-07-10 18:07 ` [PATCH 11/11] nfsd: cleanup and rename nfs4_check_open Jeff Layton
2014-07-10 20:14 ` [PATCH 00/11] nfsd: deny mode handling overhaul J. Bruce Fields
2014-07-11  7:46   ` Christoph Hellwig
2014-07-11 14:31     ` J. Bruce Fields
2014-07-11 15:42       ` Jeff Layton
2014-07-13 11:42         ` Christoph Hellwig
2014-07-13 11:52           ` Jeff Layton
2014-07-14 13:38             ` J. Bruce Fields
2014-07-15 10:00               ` Christoph Hellwig

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=1405015655-12469-10-git-send-email-jlayton@primarydata.com \
    --to=jlayton@primarydata.com \
    --cc=bfields@fieldses.org \
    --cc=hch@infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    /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