* [PATCH 1/6] SUNRPC: Fix a client regression when handling oversized replies
@ 2019-03-15 17:51 Trond Myklebust
2019-03-15 17:51 ` [PATCH 2/6] SUNRPC: Fix the minimal size for reply buffer allocation Trond Myklebust
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw)
To: linux-nfs
If the server sends a reply that is larger than the pre-allocated
buffer, then the current code may fail to register how much of
the stream that it has finished reading. This again can lead to
hangs.
Fixes: e92053a52e68 ("SUNRPC: Handle zero length fragments correctly")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
net/sunrpc/xprtsock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 42f45d33dc56..9359539907ba 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -453,7 +453,7 @@ xs_read_xdr_buf(struct socket *sock, struct msghdr *msg, int flags,
goto out;
if (ret != want)
goto out;
- } else
+ } else if (offset < seek_init)
offset = seek_init;
ret = -EMSGSIZE;
out:
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/6] SUNRPC: Fix the minimal size for reply buffer allocation 2019-03-15 17:51 [PATCH 1/6] SUNRPC: Fix a client regression when handling oversized replies Trond Myklebust @ 2019-03-15 17:51 ` Trond Myklebust 2019-03-15 17:51 ` [PATCH 3/6] SUNRPC: Use the ENOTCONN error on socket disconnect Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw) To: linux-nfs We must at minimum allocate enough memory to be able to see any auth errors in the reply from the server. Fixes: 2c94b8eca1a26 ("SUNRPC: Use au_rslack when computing reply...") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/clnt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 4216fe33204a..310873895578 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -1730,7 +1730,12 @@ call_allocate(struct rpc_task *task) req->rq_callsize = RPC_CALLHDRSIZE + (auth->au_cslack << 1) + proc->p_arglen; req->rq_callsize <<= 2; - req->rq_rcvsize = RPC_REPHDRSIZE + auth->au_rslack + proc->p_replen; + /* + * Note: the reply buffer must at minimum allocate enough space + * for the 'struct accepted_reply' from RFC5531. + */ + req->rq_rcvsize = RPC_REPHDRSIZE + auth->au_rslack + \ + max_t(size_t, proc->p_replen, 2); req->rq_rcvsize <<= 2; status = xprt->ops->buf_alloc(task); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/6] SUNRPC: Use the ENOTCONN error on socket disconnect 2019-03-15 17:51 ` [PATCH 2/6] SUNRPC: Fix the minimal size for reply buffer allocation Trond Myklebust @ 2019-03-15 17:51 ` Trond Myklebust 2019-03-15 17:51 ` [PATCH 4/6] SUNRPC: rpc_decode_header() must always return a non-zero value on error Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw) To: linux-nfs When the socket is closed, we currently send an EAGAIN error to all pending requests in order to ask them to retransmit. Use ENOTCONN instead, to ensure that they try to reconnect before attempting to transmit. This also helps SOFTCONN tasks to behave correctly in this situation. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/xprt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index e096c5a725df..d7117d241460 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -664,7 +664,7 @@ void xprt_disconnect_done(struct rpc_xprt *xprt) spin_lock_bh(&xprt->transport_lock); xprt_clear_connected(xprt); xprt_clear_write_space_locked(xprt); - xprt_wake_pending_tasks(xprt, -EAGAIN); + xprt_wake_pending_tasks(xprt, -ENOTCONN); spin_unlock_bh(&xprt->transport_lock); } EXPORT_SYMBOL_GPL(xprt_disconnect_done); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/6] SUNRPC: rpc_decode_header() must always return a non-zero value on error 2019-03-15 17:51 ` [PATCH 3/6] SUNRPC: Use the ENOTCONN error on socket disconnect Trond Myklebust @ 2019-03-15 17:51 ` Trond Myklebust 2019-03-15 17:51 ` [PATCH 5/6] SUNRPC: Handle the SYSTEM_ERR rpc error Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw) To: linux-nfs Ensure that when the "garbage args" case falls through, we do set an error of EIO. Fixes: a0584ee9aed8 ("SUNRPC: Use struct xdr_stream when decoding...") Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/clnt.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 310873895578..1f47801e0002 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -2454,7 +2454,7 @@ static noinline int rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) { struct rpc_clnt *clnt = task->tk_client; - int error = -EACCES; + int error; __be32 *p; /* RFC-1014 says that the representation of XDR data must be a @@ -2463,7 +2463,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) * undefined results */ if (task->tk_rqstp->rq_rcv_buf.len & 3) - goto out_badlen; + goto out_unparsable; p = xdr_inline_decode(xdr, 3 * sizeof(*p)); if (!p) @@ -2498,9 +2498,10 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) goto out_err; case rpc_garbage_args: trace_rpc__garbage_args(task); + error = -EIO; break; default: - trace_rpc__unparsable(task); + goto out_unparsable; } out_garbage: @@ -2514,11 +2515,6 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) rpc_exit(task, error); return error; -out_badlen: - trace_rpc__unparsable(task); - error = -EIO; - goto out_err; - out_unparsable: trace_rpc__unparsable(task); error = -EIO; @@ -2529,6 +2525,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) goto out_garbage; out_msg_denied: + error = -EACCES; p = xdr_inline_decode(xdr, sizeof(*p)); if (!p) goto out_unparsable; @@ -2540,9 +2537,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) error = -EPROTONOSUPPORT; goto out_err; default: - trace_rpc__unparsable(task); - error = -EIO; - goto out_err; + goto out_unparsable; } p = xdr_inline_decode(xdr, sizeof(*p)); @@ -2577,8 +2572,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) task->tk_xprt->servername); break; default: - trace_rpc__unparsable(task); - error = -EIO; + goto out_unparsable; } goto out_err; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/6] SUNRPC: Handle the SYSTEM_ERR rpc error 2019-03-15 17:51 ` [PATCH 4/6] SUNRPC: rpc_decode_header() must always return a non-zero value on error Trond Myklebust @ 2019-03-15 17:51 ` Trond Myklebust 2019-03-15 17:51 ` [PATCH 6/6] SUNRPC: Remove redundant check for the reply length in call_decode() Trond Myklebust 0 siblings, 1 reply; 6+ messages in thread From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw) To: linux-nfs Handle the SYSTEM_ERR rpc error by retrying the RPC call as if it were a garbage argument. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/clnt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 1f47801e0002..cb73d6c25857 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -2497,6 +2497,7 @@ rpc_decode_header(struct rpc_task *task, struct xdr_stream *xdr) error = -EOPNOTSUPP; goto out_err; case rpc_garbage_args: + case rpc_system_err: trace_rpc__garbage_args(task); error = -EIO; break; -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 6/6] SUNRPC: Remove redundant check for the reply length in call_decode() 2019-03-15 17:51 ` [PATCH 5/6] SUNRPC: Handle the SYSTEM_ERR rpc error Trond Myklebust @ 2019-03-15 17:51 ` Trond Myklebust 0 siblings, 0 replies; 6+ messages in thread From: Trond Myklebust @ 2019-03-15 17:51 UTC (permalink / raw) To: linux-nfs Now that we're using the xdr_stream functions to decode the header, the test for the minimum reply length is redundant. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- net/sunrpc/clnt.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index cb73d6c25857..228970e6e52b 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -2392,9 +2392,6 @@ 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) - goto out_retry; - xdr_init_decode(&xdr, &req->rq_rcv_buf, req->rq_rcv_buf.head[0].iov_base, req); switch (rpc_decode_header(task, &xdr)) { @@ -2405,7 +2402,6 @@ call_decode(struct rpc_task *task) task->tk_pid, __func__, task->tk_status); return; case -EAGAIN: -out_retry: task->tk_status = 0; /* Note: rpc_decode_header() may have freed the RPC slot */ if (task->tk_rqstp == req) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-15 17:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-15 17:51 [PATCH 1/6] SUNRPC: Fix a client regression when handling oversized replies Trond Myklebust 2019-03-15 17:51 ` [PATCH 2/6] SUNRPC: Fix the minimal size for reply buffer allocation Trond Myklebust 2019-03-15 17:51 ` [PATCH 3/6] SUNRPC: Use the ENOTCONN error on socket disconnect Trond Myklebust 2019-03-15 17:51 ` [PATCH 4/6] SUNRPC: rpc_decode_header() must always return a non-zero value on error Trond Myklebust 2019-03-15 17:51 ` [PATCH 5/6] SUNRPC: Handle the SYSTEM_ERR rpc error Trond Myklebust 2019-03-15 17:51 ` [PATCH 6/6] SUNRPC: Remove redundant check for the reply length in call_decode() Trond Myklebust
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).