diff mbox series

[SRU,Disco,2/2] SUNRPC: Fix a use after free when a server rejects the RPCSEC_GSS credential

Message ID 20191030035006.31696-3-matthew.ruffell@canonical.com
State New
Headers show
Series SUNRPC: Use after free when GSSD credentials are invalid causes oops | expand

Commit Message

Matthew Ruffell Oct. 30, 2019, 3:50 a.m. UTC 
From: Trond Myklebust <trond.myklebust@hammerspace.com>

BugLink: https://bugs.launchpad.net/bugs/1842037

The addition of rpc_check_timeout() to call_decode causes an Oops
when the RPCSEC_GSS credential is rejected.
The reason is that rpc_decode_header() will call xprt_release() in
order to free task->tk_rqstp, which is needed by rpc_check_timeout()
to check whether or not we should exit due to a soft timeout.

The fix is to move the call to xprt_release() into call_decode() so
we can perform it after rpc_check_timeout().

Reported-by: Olga Kornievskaia <olga.kornievskaia@gmail.com>
Reported-by: Nick Bowler <nbowler@draconx.ca>
Fixes: cea57789e408 ("SUNRPC: Clean up")
Cc: stable@vger.kernel.org # v5.1+
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
(backported from commit 7987b694ade8cc465ce10fb3dceaa614f13ceaf3)
[mruffell: rewrite goto error handling, medium context adjustments]
Signed-off-by: Matthew Ruffell <matthew.ruffell@canonical.com>
---
 net/sunrpc/clnt.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index f9568b0dc63e..5ea3c62fff9f 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -2303,6 +2303,8 @@  call_decode(struct rpc_task *task)
 	if (IS_ERR(p)) {
 		if (p == ERR_PTR(-EAGAIN))
 			goto out_retry;
+		if (p == ERR_PTR(-EKEYREJECTED))
+			goto out_key_rejected;
 		return;
 	}
 	task->tk_action = rpc_exit_task;
@@ -2315,17 +2317,21 @@  call_decode(struct rpc_task *task)
 	return;
 out_retry:
 	task->tk_status = 0;
-	/* Note: rpc_verify_header() may have freed the RPC slot */
-	if (task->tk_rqstp == req) {
-		xdr_free_bvec(&req->rq_rcv_buf);
-		req->rq_reply_bytes_recvd = 0;
-		req->rq_rcv_buf.len = 0;
-		if (task->tk_client->cl_discrtry)
-			xprt_conditional_disconnect(req->rq_xprt,
-						    req->rq_connect_cookie);
-	}
+	xdr_free_bvec(&req->rq_rcv_buf);
+	req->rq_reply_bytes_recvd = 0;
+	req->rq_rcv_buf.len = 0;
+	if (task->tk_client->cl_discrtry)
+		xprt_conditional_disconnect(req->rq_xprt,
+					    req->rq_connect_cookie);
 	task->tk_action = call_encode;
 	rpc_check_timeout(task);
+	return;
+out_key_rejected:
+	task->tk_action = call_reserve;
+	rpc_check_timeout(task);
+	rpcauth_invalcred(task);
+	/* Ensure we obtain a new XID if we retry! */
+	xprt_release(task);
 }
 
 static __be32 *
@@ -2413,11 +2419,7 @@  rpc_verify_header(struct rpc_task *task)
 			task->tk_cred_retry--;
 			dprintk("RPC: %5u %s: retry stale creds\n",
 					task->tk_pid, __func__);
-			rpcauth_invalcred(task);
-			/* Ensure we obtain a new XID! */
-			xprt_release(task);
-			task->tk_action = call_reserve;
-			goto out_retry;
+			return ERR_PTR(-EKEYREJECTED);
 		case RPC_AUTH_BADCRED:
 		case RPC_AUTH_BADVERF:
 			/* possibly garbled cred/verf? */