linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).