Skip to contentSkip to conversation

nfs4: fix comments in discover_server_trunking

PATCH In review Weston Andros Adamson 2 participants 4 messages Jan 20, 2014 – Jan 21, 2014 1 message not in the archive

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Weston Andros Adamson

author
#1 lore raw

PATCH 2/2 nfs4: fix comments in discover_server_trunking

'result' is not updated if a match isn't found in the nfs4_discover_server_trunking path.

Signed-off-by Weston Andros Adamson <[email protected]>
---
 fs/nfs/nfs4client.c | 4 ++--
 fs/nfs/nfs4state.c  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
fs/nfs/nfs4client.c +2 -2
index 0f9b772..147a752 100644
@@ -465,7 +465,7 @@ static void nfs4_swap_callback_idents(struct nfs_client *keep,
  * nfs40_walk_client_list - Find server that recognizes a client ID
  *
  * @new: nfs_client with client ID to test
- * @result: OUT: found nfs_client, or new
+ * @result: OUT: found nfs_client
  * @cred: credential to use for trunking test
  *
  * Returns zero, a negative errno, or a negative NFS4ERR status.

@@ -601,7 +601,7 @@ out_major_mismatch:
  * nfs41_walk_client_list - Find nfs_client that matches a client/server owner
  *
  * @new: nfs_client with client ID to test
- * @result: OUT: found nfs_client, or new
+ * @result: OUT: found nfs_client
  * @cred: credential to use for trunking test
  *
  * Returns zero, a negative errno, or a negative NFS4ERR status.
fs/nfs/nfs4state.c +1 -1
index e5be725..666512e 100644
@@ -328,7 +328,7 @@ out:
  * nfs41_discover_server_trunking - Detect server IP address trunking (mv1)
  *
  * @clp: nfs_client under test
- * @result: OUT: found nfs_client, or clp
+ * @result: OUT: found nfs_client
  * @cred: credential to use for trunking test
  *
  * Returns NFS4_OK, a negative errno, or a negative NFS4ERR status.
Signature · 1 line
1.8.3.4 (Apple Git-47)
author
#2 lore raw

Ignore this patch. I misunderstood the meaning of the original comments...

-dros

Quoted 47 lines · Weston Andros Adamson

'result' is not updated if a match isn't found in the nfs4_discover_server_trunking path.

Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/nfs4client.c | 4 ++--
fs/nfs/nfs4state.c | 2 +-

2 files changed, 3 insertions(+), 3 deletions(-)
fs/nfs/nfs4client.c +2 -2
index 0f9b772..147a752 100644
@@ -465,7 +465,7 @@ static void nfs4_swap_callback_idents(struct nfs_client *keep,
 * nfs40_walk_client_list - Find server that recognizes a client ID
 *
 * @new: nfs_client with client ID to test
- * @result: OUT: found nfs_client, or new
+ * @result: OUT: found nfs_client
 * @cred: credential to use for trunking test
 *
 * Returns zero, a negative errno, or a negative NFS4ERR status.

@@ -601,7 +601,7 @@ out_major_mismatch:
 * nfs41_walk_client_list - Find nfs_client that matches a client/server owner
 *
 * @new: nfs_client with client ID to test
- * @result: OUT: found nfs_client, or new
+ * @result: OUT: found nfs_client
 * @cred: credential to use for trunking test
 *
 * Returns zero, a negative errno, or a negative NFS4ERR status.
fs/nfs/nfs4state.c +1 -1
index e5be725..666512e 100644
@@ -328,7 +328,7 @@ out:
 * nfs41_discover_server_trunking - Detect server IP address trunking (mv1)
 *
 * @clp: nfs_client under test
- * @result: OUT: found nfs_client, or clp
+ * @result: OUT: found nfs_client
 * @cred: credential to use for trunking test
 *
 * Returns NFS4_OK, a negative errno, or a negative NFS4ERR status.
Signature · 1 line
1.8.3.4 (Apple Git-47)
Applied
#3 lore raw

Re: [PATCH 1/2] nfs4: fix discover_server_trunking use after free

In reply to a message not in the archive
Quoted 18 lines · Weston Andros Adamson

If clp is new (cl_count = 1) and it matches another client in nfs4_discover_server_trunking, the nfs_put_client will free clp before ->cl_preserve_clid is set.

Cc [email protected] # 3.7+
Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/nfs4client.c | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfs/nfs4client.c +0 -0
index c1b7a80..0f9b772 100644
@@ -414,11 +414,12 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 	error = nfs4_discover_server_trunking(clp, &old);
 	if (error < 0)
 		goto error;

fs/nfs/nfs4client.c struct nfs_client *nfs4_init_client(struct nfs_client *clp, · line 423

Quoted 10 lines
fs/nfs/nfs4client.c +4 -3
-	nfs_put_client(clp);
-	if (clp != old) {
+
+	if (clp != old)
 		clp->cl_preserve_clid = true;
+	nfs_put_client(clp);
+	if (clp != old)
 		clp = old;
-	}

Applied, but I cleaned it up by replacing the last 2 lines with an equivalent ‘return old’...

--
Trond Myklebust
Linux NFS client maintainer

author
#4 lore raw

Re: [PATCH 1/2] nfs4: fix discover_server_trunking use after free

Quoted 25 lines · Trond Myklebust
Quoted 22 lines · Weston Andros Adamson

If clp is new (cl_count = 1) and it matches another client in nfs4_discover_server_trunking, the nfs_put_client will free clp before ->cl_preserve_clid is set.

Cc [email protected] # 3.7+
Signed-off-by Weston Andros Adamson <[email protected]>

---
fs/nfs/nfs4client.c | 7 ++++---

1 file changed, 4 insertions(+), 3 deletions(-)
fs/nfs/nfs4client.c +2 -2
index c1b7a80..0f9b772 100644
@@ -414,11 +414,12 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
 	error = nfs4_discover_server_trunking(clp, &old);
 	if (error < 0)
 		goto error;
-	nfs_put_client(clp);
-	if (clp != old) {
+
+	if (clp != old)
Quoted 8 lines

fs/nfs/nfs4client.c struct nfs_client *nfs4_init_client(struct nfs_client *clp, · line 423

Quoted 6 lines
fs/nfs/nfs4client.c +2 -1
 		clp->cl_preserve_clid = true;
+	nfs_put_client(clp);
+	if (clp != old)
 		clp = old;
-	}

Applied, but I cleaned it up by replacing the last 2 lines with an equivalent ‘return old’…

Cool, I almost posted that, but thought this change was a “cleaner” diff.

-dros

Quoted 9 lines

--
Trond Myklebust
Linux NFS client maintainer

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html