| Message ID | 20161029021531.2673.6849.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2016-10-28 at 22:22 -0400, Chuck Lever wrote: > The underlying transport releases the page pointed to by rq_buffer > during xprt_rdma_bc_send_request. When the backchannel reply arrives, > rq_rbuffer then points to freed memory. > > Fixes: 68778945e46f ('SUNRPC: Separate buffer pointers for RPC ...') > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > Cc: Jeff Layton <jlayton@redhat.com> > --- > net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > Hi Bruce- > > This applies on top of Jeff's recent patch in the same area. It's > an obvious quick fix rather than a deep review of that code path. > > I've tested with iozone, git "make test", and some xfstests with > NFSv4.1 / RDMA; I ran into another crasher that is preventing more > extensive testing. The prepare_creds crash has not re-appeared so > far. > > I enabled RPC client debugging on the server during these tests to > confirm that the CB_RECALL operations were successful. > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > index fc4535e..20027f8 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c > @@ -177,19 +177,26 @@ static int svc_rdma_bc_sendto(struct svcxprt_rdma *rdma, > return -EINVAL; > } > > + /* svc_rdma_sendto releases this page */ > page = alloc_page(RPCRDMA_DEF_GFP); > if (!page) > return -ENOMEM; > - > rqst->rq_buffer = page_address(page); > - rqst->rq_rbuffer = (char *)rqst->rq_buffer + rqst->rq_callsize; > + > + rqst->rq_rbuffer = kmalloc(rqst->rq_rcvsize, RPCRDMA_DEF_GFP); > + if (!rqst->rq_rbuffer) { > + put_page(page); > + return -ENOMEM; > + } > return 0; > } > > static void > xprt_rdma_bc_free(struct rpc_task *task) > { > - /* No-op: ctxt and page have already been freed. */ > + struct rpc_rqst *rqst = task->tk_rqstp; > + > + kfree(rqst->rq_rbuffer); > } > > static int > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Obviously, mine was not based on any deep reading of this code either, so I'll take your word for it on this: Acked-by: Jeff Layton <jlayton@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c index fc4535e..20027f8 100644 --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c @@ -177,19 +177,26 @@ static int svc_rdma_bc_sendto(struct svcxprt_rdma *rdma, return -EINVAL; } + /* svc_rdma_sendto releases this page */ page = alloc_page(RPCRDMA_DEF_GFP); if (!page) return -ENOMEM; - rqst->rq_buffer = page_address(page); - rqst->rq_rbuffer = (char *)rqst->rq_buffer + rqst->rq_callsize; + + rqst->rq_rbuffer = kmalloc(rqst->rq_rcvsize, RPCRDMA_DEF_GFP); + if (!rqst->rq_rbuffer) { + put_page(page); + return -ENOMEM; + } return 0; } static void xprt_rdma_bc_free(struct rpc_task *task) { - /* No-op: ctxt and page have already been freed. */ + struct rpc_rqst *rqst = task->tk_rqstp; + + kfree(rqst->rq_rbuffer); } static int
The underlying transport releases the page pointed to by rq_buffer during xprt_rdma_bc_send_request. When the backchannel reply arrives, rq_rbuffer then points to freed memory. Fixes: 68778945e46f ('SUNRPC: Separate buffer pointers for RPC ...') Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: Jeff Layton <jlayton@redhat.com> --- net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) Hi Bruce- This applies on top of Jeff's recent patch in the same area. It's an obvious quick fix rather than a deep review of that code path. I've tested with iozone, git "make test", and some xfstests with NFSv4.1 / RDMA; I ran into another crasher that is preventing more extensive testing. The prepare_creds crash has not re-appeared so far. I enabled RPC client debugging on the server during these tests to confirm that the CB_RECALL operations were successful. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html