From: bfields@fieldses.org (J. Bruce Fields)
To: Trond Myklebust <trond.myklebust@primarydata.com>,
Anna Schumaker <schumakeranna@gmail.com>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH] nfs: fix false positives in nfs40_walk_client_list()
Date: Mon, 28 Nov 2016 09:02:52 -0500 [thread overview]
Message-ID: <20161128140252.GA28629@fieldses.org> (raw)
From: "J. Bruce Fields" <bfields@redhat.com>
It's possible that two different servers can return the same (clientid,
verifier) pair purely by coincidence. Both are 64-bit values, but
depending on the server implementation, they can be highly predictable
and collisions may be quite likely, especially when there are lots of
servers.
So, check for this case. If the clientid and verifier both match, then
we actually know they *can't* be the same server, since a new
SETCLIENTID to an already-known server should have changed the verifier.
This helps fix a bug that could cause the client to mount a filesystem
from the wrong server.
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Tested-by: Yongcheng Yang <yoyang@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfs/nfs4client.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 074ac7131459..5e2747644432 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -464,6 +464,11 @@ static bool nfs4_match_client_owner_id(const struct nfs_client *clp1,
return strcmp(clp1->cl_owner_id, clp2->cl_owner_id) == 0;
}
+static bool nfs4_same_verifier(nfs4_verifier *v1, nfs4_verifier *v2)
+{
+ return 0 == memcmp(v1->data, v2->data, sizeof(v1->data));
+}
+
/**
* nfs40_walk_client_list - Find server that recognizes a client ID
*
@@ -521,7 +526,21 @@ int nfs40_walk_client_list(struct nfs_client *new,
if (!nfs4_match_client_owner_id(pos, new))
continue;
-
+ /*
+ * We just sent a new SETCLIENTID, which should have
+ * caused the server to return a new cl_confirm. So if
+ * cl_confirm is the same, then this is a different
+ * server that just returned the same cl_confirm by
+ * coincidence:
+ */
+ if ((new != pos) && nfs4_same_verifier(&pos->cl_confirm,
+ &new->cl_confirm))
+ continue;
+ /*
+ * But if the cl_confirm's are different, then the only
+ * way that a SETCLIENTID_CONFIRM to pos can succeed is
+ * if new and pos point to the same server:
+ */
atomic_inc(&pos->cl_count);
spin_unlock(&nn->nfs_client_lock);
@@ -534,6 +553,7 @@ int nfs40_walk_client_list(struct nfs_client *new,
break;
case 0:
nfs4_swap_callback_idents(pos, new);
+ pos->cl_confirm = new->cl_confirm;
prev = NULL;
*result = pos;
--
2.9.3
next reply other threads:[~2016-11-28 14:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-28 14:02 J. Bruce Fields [this message]
2016-11-28 18:46 ` [PATCH] nfs: fix false positives in nfs40_walk_client_list() Anna Schumaker
2016-11-28 19:23 ` J. Bruce Fields
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=20161128140252.GA28629@fieldses.org \
--to=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
--cc=schumakeranna@gmail.com \
--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).