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: Trond Myklebust <Trond.Myklebust@netapp.com>

2 files changed, 15 insertions, 14 deletions

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 522b06849f86..3ae560464513 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1199,18 +1199,6 @@ call_decode(struct rpc_task *task)
                 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()
          * before it changed req->rq_received.
@@ -1222,6 +1210,18 @@ call_decode(struct rpc_task *task)
         WARN_ON(memcmp(&req->rq_rcv_buf, &req->rq_private_buf,
                                 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 */
         p = call_verify(task);
         if (IS_ERR(p)) {
@@ -1243,7 +1243,7 @@ out_retry:
         task->tk_status = 0;
         /* Note: call_verify() may have freed the RPC slot */
         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)
                         xprt_force_disconnect(task->tk_xprt);
         }
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 3ba64f9f84ba..5110a4ea7fdf 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -757,9 +757,10 @@ void xprt_complete_rqst(struct rpc_task *task, int copied)
         task->tk_rtt = (long)jiffies - req->rq_xtime;
 
         list_del_init(&req->rq_list);
+        req->rq_private_buf.len = copied;
         /* Ensure all writes are done before we update req->rq_received */
         smp_wmb();
-        req->rq_received = req->rq_private_buf.len = copied;
+        req->rq_received = copied;
         rpc_wake_up_queued_task(&xprt->pending, task);
 }
 EXPORT_SYMBOL_GPL(xprt_complete_rqst);