xprtrdma: Replace use of xdr_stream_pos in rpcrdma_marshal_req
authorChuck Lever <chuck.lever@oracle.com>
Wed, 19 Jun 2019 14:32:43 +0000 (10:32 -0400)
committerAnna Schumaker <Anna.Schumaker@Netapp.com>
Tue, 9 Jul 2019 14:30:11 +0000 (10:30 -0400)
This is a latent bug. xdr_stream_pos works by subtracting
xdr_stream::nwords from xdr_buf::len. But xdr_stream::nwords is not
initialized by xdr_init_encode().

It works today only because all fields in rpcrdma_req::rl_stream
are initialized to zero by rpcrdma_req_create, making the
subtraction in xdr_stream_pos always a no-op.

I found this issue via code inspection. It was introduced by commit
39f4cd9e9982 ("xprtrdma: Harden chunk list encoding against send
buffer overflow"), but the code has changed enough since then that
this fix can't be automatically applied to stable.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
include/trace/events/rpcrdma.h
net/sunrpc/xprtrdma/rpc_rdma.c

index f0678e3..59492a9 100644 (file)
@@ -470,13 +470,12 @@ TRACE_DEFINE_ENUM(rpcrdma_replych);
 
 TRACE_EVENT(xprtrdma_marshal,
        TP_PROTO(
-               const struct rpc_rqst *rqst,
-               unsigned int hdrlen,
+               const struct rpcrdma_req *req,
                unsigned int rtype,
                unsigned int wtype
        ),
 
-       TP_ARGS(rqst, hdrlen, rtype, wtype),
+       TP_ARGS(req, rtype, wtype),
 
        TP_STRUCT__entry(
                __field(unsigned int, task_id)
@@ -491,10 +490,12 @@ TRACE_EVENT(xprtrdma_marshal,
        ),
 
        TP_fast_assign(
+               const struct rpc_rqst *rqst = &req->rl_slot;
+
                __entry->task_id = rqst->rq_task->tk_pid;
                __entry->client_id = rqst->rq_task->tk_client->cl_clid;
                __entry->xid = be32_to_cpu(rqst->rq_xid);
-               __entry->hdrlen = hdrlen;
+               __entry->hdrlen = req->rl_hdrbuf.len;
                __entry->headlen = rqst->rq_snd_buf.head[0].iov_len;
                __entry->pagelen = rqst->rq_snd_buf.page_len;
                __entry->taillen = rqst->rq_snd_buf.tail[0].iov_len;
index 85115a2..97bfb80 100644 (file)
@@ -867,12 +867,12 @@ rpcrdma_marshal_req(struct rpcrdma_xprt *r_xprt, struct rpc_rqst *rqst)
        if (ret)
                goto out_err;
 
-       trace_xprtrdma_marshal(rqst, xdr_stream_pos(xdr), rtype, wtype);
-
-       ret = rpcrdma_prepare_send_sges(r_xprt, req, xdr_stream_pos(xdr),
+       ret = rpcrdma_prepare_send_sges(r_xprt, req, req->rl_hdrbuf.len,
                                        &rqst->rq_snd_buf, rtype);
        if (ret)
                goto out_err;
+
+       trace_xprtrdma_marshal(req, rtype, wtype);
        return 0;
 
 out_err: