2019-06-25 03:54:19

by Trond Myklebust

Subject: [PATCH] SUNRPC: Fix up calculation of client message length

In the case where a record marker was used, xs_sendpages() needs
to return the length of the payload + record marker so that we
operate correctly in the case of a partial transmission.
When the callers check return value, they therefore need to
take into account the record marker length.

Fixes: 06b5fc3ad94e ("Merge tag 'nfs-rdma-for-5.1-1'...")
Cc: [email protected] # 5.1+
Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/xprtsock.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index d7b8e95a61c8..97c15d47f343 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -950,6 +950,8 @@ static int xs_local_send_request(struct rpc_rqst *req)
struct sock_xprt *transport =
container_of(xprt, struct sock_xprt, xprt);
struct xdr_buf *xdr = &req->rq_snd_buf;
+ rpc_fraghdr rm = xs_stream_record_marker(xdr);
+ unsigned int msglen = rm ? req->rq_slen + sizeof(rm) : req->rq_slen;
int status;
int sent = 0;

@@ -964,9 +966,7 @@ static int xs_local_send_request(struct rpc_rqst *req)

req->rq_xtime = ktime_get();
status = xs_sendpages(transport->sock, NULL, 0, xdr,
- transport->xmit.offset,
- xs_stream_record_marker(xdr),
- &sent);
+ transport->xmit.offset, rm, &sent);
dprintk("RPC: %s(%u) = %d\n",
__func__, xdr->len - transport->xmit.offset, status);

@@ -976,7 +976,7 @@ static int xs_local_send_request(struct rpc_rqst *req)
if (likely(sent > 0) || status == 0) {
transport->xmit.offset += sent;
req->rq_bytes_sent = transport->xmit.offset;
- if (likely(req->rq_bytes_sent >= req->rq_slen)) {
+ if (likely(req->rq_bytes_sent >= msglen)) {
req->rq_xmit_bytes_sent += transport->xmit.offset;
transport->xmit.offset = 0;
return 0;
@@ -1097,6 +1097,8 @@ static int xs_tcp_send_request(struct rpc_rqst *req)
struct rpc_xprt *xprt = req->rq_xprt;
struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
struct xdr_buf *xdr = &req->rq_snd_buf;
+ rpc_fraghdr rm = xs_stream_record_marker(xdr);
+ unsigned int msglen = rm ? req->rq_slen + sizeof(rm) : req->rq_slen;
bool vm_wait = false;
int status;
int sent;
@@ -1122,9 +1124,7 @@ static int xs_tcp_send_request(struct rpc_rqst *req)
while (1) {
sent = 0;
status = xs_sendpages(transport->sock, NULL, 0, xdr,
- transport->xmit.offset,
- xs_stream_record_marker(xdr),
- &sent);
+ transport->xmit.offset, rm, &sent);

dprintk("RPC: xs_tcp_send_request(%u) = %d\n",
xdr->len - transport->xmit.offset, status);
@@ -1133,7 +1133,7 @@ static int xs_tcp_send_request(struct rpc_rqst *req)
* reset the count of bytes sent. */
transport->xmit.offset += sent;
req->rq_bytes_sent = transport->xmit.offset;
- if (likely(req->rq_bytes_sent >= req->rq_slen)) {
+ if (likely(req->rq_bytes_sent >= msglen)) {
req->rq_xmit_bytes_sent += transport->xmit.offset;
transport->xmit.offset = 0;
return 0;
--
2.21.0

2019-06-25 03:57:56

by Trond Myklebust

Subject: Re: [PATCH] SUNRPC: Fix up calculation of client message length

Hi Anna,

On Mon, 2019-06-24 at 19:15 -0400, Trond Myklebust wrote:
> In the case where a record marker was used, xs_sendpages() needs
> to return the length of the payload + record marker so that we
> operate correctly in the case of a partial transmission.
> When the callers check return value, they therefore need to
> take into account the record marker length.
>
> Fixes: 06b5fc3ad94e ("Merge tag 'nfs-rdma-for-5.1-1'...")
> Cc: [email protected] # 5.1+
> Signed-off-by: Trond Myklebust <[email protected]>
>

This patch fixes an issue which is causing the NFS server to close the
TCP connection every few moments due to incorrectly formatted RPC
messages. It is also capable of corrupting data, so please apply as
soon as possible.

Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]