Commit 1e799b67 authored by Trond Myklebust's avatar Trond Myklebust

SUNRPC: Fix read ordering problems with req->rq_private_buf.len

We want to ensure that req->rq_private_buf.len is updated before
req->rq_received, so that call_decode() doesn't use an old value for
req->rq_rcv_buf.len.

In 'call_decode()' itself, instead of using task->tk_status (which is set
using req->rq_received) must use the actual value of
req->rq_private_buf.len when deciding whether or not the received RPC reply
is too short.

Finally ensure that we set req->rq_rcv_buf.len to zero when retrying a
request. A typo meant that we were resetting req->rq_private_buf.len in
call_decode(), and then clobbering that value with the old rq_rcv_buf.len
again in xprt_transmit().
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent c1d51931
...@@ -1199,18 +1199,6 @@ call_decode(struct rpc_task *task) ...@@ -1199,18 +1199,6 @@ call_decode(struct rpc_task *task)
task->tk_flags &= ~RPC_CALL_MAJORSEEN; task->tk_flags &= ~RPC_CALL_MAJORSEEN;
} }
if (task->tk_status < 12) {
if (!RPC_IS_SOFT(task)) {
task->tk_action = call_bind;
clnt->cl_stats->rpcretrans++;
goto out_retry;
}
dprintk("RPC: %s: too small RPC reply size (%d bytes)\n",
clnt->cl_protname, task->tk_status);
task->tk_action = call_timeout;
goto out_retry;
}
/* /*
* Ensure that we see all writes made by xprt_complete_rqst() * Ensure that we see all writes made by xprt_complete_rqst()
* before it changed req->rq_received. * before it changed req->rq_received.
...@@ -1222,6 +1210,18 @@ call_decode(struct rpc_task *task) ...@@ -1222,6 +1210,18 @@ call_decode(struct rpc_task *task)
WARN_ON(memcmp(&req->rq_rcv_buf, &req->rq_private_buf, WARN_ON(memcmp(&req->rq_rcv_buf, &req->rq_private_buf,
sizeof(req->rq_rcv_buf)) != 0); sizeof(req->rq_rcv_buf)) != 0);
if (req->rq_rcv_buf.len < 12) {
if (!RPC_IS_SOFT(task)) {
task->tk_action = call_bind;
clnt->cl_stats->rpcretrans++;
goto out_retry;
}
dprintk("RPC: %s: too small RPC reply size (%d bytes)\n",
clnt->cl_protname, task->tk_status);
task->tk_action = call_timeout;
goto out_retry;
}
/* Verify the RPC header */ /* Verify the RPC header */
p = call_verify(task); p = call_verify(task);
if (IS_ERR(p)) { if (IS_ERR(p)) {
...@@ -1243,7 +1243,7 @@ call_decode(struct rpc_task *task) ...@@ -1243,7 +1243,7 @@ call_decode(struct rpc_task *task)
task->tk_status = 0; task->tk_status = 0;
/* Note: call_verify() may have freed the RPC slot */ /* Note: call_verify() may have freed the RPC slot */
if (task->tk_rqstp == req) { if (task->tk_rqstp == req) {
req->rq_received = req->rq_private_buf.len = 0; req->rq_received = req->rq_rcv_buf.len = 0;
if (task->tk_client->cl_discrtry) if (task->tk_client->cl_discrtry)
xprt_force_disconnect(task->tk_xprt); xprt_force_disconnect(task->tk_xprt);
} }
......
...@@ -757,9 +757,10 @@ void xprt_complete_rqst(struct rpc_task *task, int copied) ...@@ -757,9 +757,10 @@ void xprt_complete_rqst(struct rpc_task *task, int copied)
task->tk_rtt = (long)jiffies - req->rq_xtime; task->tk_rtt = (long)jiffies - req->rq_xtime;
list_del_init(&req->rq_list); list_del_init(&req->rq_list);
req->rq_private_buf.len = copied;
/* Ensure all writes are done before we update req->rq_received */ /* Ensure all writes are done before we update req->rq_received */
smp_wmb(); smp_wmb();
req->rq_received = req->rq_private_buf.len = copied; req->rq_received = copied;
rpc_wake_up_queued_task(&xprt->pending, task); rpc_wake_up_queued_task(&xprt->pending, task);
} }
EXPORT_SYMBOL_GPL(xprt_complete_rqst); EXPORT_SYMBOL_GPL(xprt_complete_rqst);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment