[PATCH 5.4 0/4] Request to cherry-pick f00432063db1 to 5.4.y

[PATCH 5.4 0/4] Request to cherry-pick f00432063db1 to 5.4.y

[Meena Shanmugam]

The commit f00432063db1a0db484e85193eccc6845435b80e (SUNRPC:
Ensure we flush any closed sockets before xs_xprt_free()) upstream fixes
CVE-2022-28893, hence good candidate for stable trees.
The above commit depends on 3be232f11a3c (SUNRPC: Prevent immediate
close+reconnect)  and  89f42494f92f (SUNRPC: Don't call connect() more than
once on a TCP socket). Commit 3be232f11a3c depends on commit
e26d9972720e (SUNRPC: Clean up scheduling of autoclose).

Commits e26d9972720e, 3be232f11a3c apply cleanly on 5.4
kernel. commit 89f42494f92f and f00432063db1 didn't apply cleanly.
This patch series includes all the commits required for back porting
f00432063db1.


Trond Myklebust (4):
  SUNRPC: Clean up scheduling of autoclose
  SUNRPC: Prevent immediate close+reconnect
  SUNRPC: Don't call connect() more than once on a TCP socket
  SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()

 fs/file_table.c                 |  1 +
 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprt.c               | 34 ++++++++++++++++---------------
 net/sunrpc/xprtsock.c           | 36 ++++++++++++++++++++++-----------
 4 files changed, 44 insertions(+), 28 deletions(-)

-- 
2.36.1.124.g0e6072fb45-goog

[PATCH 5.4 1/4] SUNRPC: Clean up scheduling of autoclose

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit e26d9972720e2484f44cdd94ca4e31cc372ed2ed upstream.

Consolidate duplicated code in xprt_force_disconnect() and
xprt_conditional_disconnect().

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 net/sunrpc/xprt.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 8ac579778e48..a7dedc12c982 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -717,21 +717,29 @@ void xprt_disconnect_done(struct rpc_xprt *xprt)
 EXPORT_SYMBOL_GPL(xprt_disconnect_done);
 
 /**
- * xprt_force_disconnect - force a transport to disconnect
+ * xprt_schedule_autoclose_locked - Try to schedule an autoclose RPC call
  * @xprt: transport to disconnect
- *
  */
-void xprt_force_disconnect(struct rpc_xprt *xprt)
+static void xprt_schedule_autoclose_locked(struct rpc_xprt *xprt)
 {
-	/* Don't race with the test_bit() in xprt_clear_locked() */
-	spin_lock(&xprt->transport_lock);
 	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
-	/* Try to schedule an autoclose RPC call */
 	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
 		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
 	else if (xprt->snd_task && !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
 		rpc_wake_up_queued_task_set_status(&xprt->pending,
 						   xprt->snd_task, -ENOTCONN);
+}
+
+/**
+ * xprt_force_disconnect - force a transport to disconnect
+ * @xprt: transport to disconnect
+ *
+ */
+void xprt_force_disconnect(struct rpc_xprt *xprt)
+{
+	/* Don't race with the test_bit() in xprt_clear_locked() */
+	spin_lock(&xprt->transport_lock);
+	xprt_schedule_autoclose_locked(xprt);
 	spin_unlock(&xprt->transport_lock);
 }
 EXPORT_SYMBOL_GPL(xprt_force_disconnect);
@@ -771,11 +779,7 @@ void xprt_conditional_disconnect(struct rpc_xprt *xprt, unsigned int cookie)
 		goto out;
 	if (test_bit(XPRT_CLOSING, &xprt->state))
 		goto out;
-	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
-	/* Try to schedule an autoclose RPC call */
-	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
-		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
-	xprt_wake_pending_tasks(xprt, -EAGAIN);
+	xprt_schedule_autoclose_locked(xprt);
 out:
 	spin_unlock(&xprt->transport_lock);
 }
-- 
2.36.1.124.g0e6072fb45-goog

[PATCH 5.4 2/4] SUNRPC: Prevent immediate close+reconnect

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 3be232f11a3cc9b0ef0795e39fa11bdb8e422a06 upstream.

If we have already set up the socket and are waiting for it to connect,
then don't immediately close and retry.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 net/sunrpc/xprt.c     | 3 ++-
 net/sunrpc/xprtsock.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index a7dedc12c982..68d08dcba018 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -722,7 +722,8 @@ EXPORT_SYMBOL_GPL(xprt_disconnect_done);
  */
 static void xprt_schedule_autoclose_locked(struct rpc_xprt *xprt)
 {
-	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
+	if (test_and_set_bit(XPRT_CLOSE_WAIT, &xprt->state))
+		return;
 	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
 		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
 	else if (xprt->snd_task && !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 43bc02dea80c..49ba817f4fb6 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2469,7 +2469,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL) {
+	if (transport->sock != NULL && !xprt_connecting(xprt)) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
 				"seconds\n",
 				xprt, xprt->reestablish_timeout / HZ);
-- 
2.36.1.124.g0e6072fb45-goog

[PATCH 5.4 3/4] SUNRPC: Don't call connect() more than once on a TCP socket

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.

Avoid socket state races due to repeated calls to ->connect() using the
same socket. If connect() returns 0 due to the connection having
completed, but we are in fact in a closing state, then we may leave the
XPRT_CONNECTING flag set on the transport.

Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Fix merge conflict in xs_tcp_setup_socket]
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 Added fallthrough which I missed in 5.10 patch.

 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprtsock.c           | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index a940de03808d..46deca97e806 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -90,6 +90,7 @@ struct sock_xprt {
 #define XPRT_SOCK_WAKE_WRITE	(5)
 #define XPRT_SOCK_WAKE_PENDING	(6)
 #define XPRT_SOCK_WAKE_DISCONNECT	(7)
+#define XPRT_SOCK_CONNECT_SENT	(8)
 
 #endif /* __KERNEL__ */
 
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 49ba817f4fb6..29e9c54a89d3 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2384,10 +2384,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 	struct rpc_xprt *xprt = &transport->xprt;
 	int status = -EIO;
 
-	if (!sock) {
-		sock = xs_create_sock(xprt, transport,
-				xs_addr(xprt)->sa_family, SOCK_STREAM,
-				IPPROTO_TCP, true);
+	if (xprt_connected(xprt))
+		goto out;
+	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
+			       &transport->sock_state) ||
+	    !sock) {
+		xs_reset_transport(transport);
+		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
+				      SOCK_STREAM, IPPROTO_TCP, true);
 		if (IS_ERR(sock)) {
 			status = PTR_ERR(sock);
 			goto out;
@@ -2418,6 +2422,8 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 		break;
 	case 0:
 	case -EINPROGRESS:
+		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
+		fallthrough;
 	case -EALREADY:
 		xprt_unlock_connect(xprt, transport);
 		return;
@@ -2469,13 +2475,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL && !xprt_connecting(xprt)) {
+	if (transport->sock != NULL) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
-				"seconds\n",
-				xprt, xprt->reestablish_timeout / HZ);
-
-		/* Start by resetting any existing state */
-		xs_reset_transport(transport);
+			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
 
 		delay = xprt_reconnect_delay(xprt);
 		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
-- 
2.36.1.124.g0e6072fb45-goog

[PATCH 5.4 4/4] SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit f00432063db1a0db484e85193eccc6845435b80e upstream.

We must ensure that all sockets are closed before we call xprt_free()
and release the reference to the net namespace. The problem is that
calling fput() will defer closing the socket until delayed_fput() gets
called.
Let's fix the situation by allowing rpciod and the transport teardown
code (which runs on the system wq) to call __fput_sync(), and directly
close the socket.

Reported-by: Felix Fu <foyjog@gmail.com>
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Fixes: a73881c96d73 ("SUNRPC: Fix an Oops in udp_poll()")
Cc: stable@vger.kernel.org # 5.1.x: 3be232f11a3c: SUNRPC: Prevent immediate close+reconnect
Cc: stable@vger.kernel.org # 5.1.x: 89f42494f92f: SUNRPC: Don't call connect() more than once on a TCP socket
Cc: stable@vger.kernel.org # 5.1.x
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Fix merge conflict in xprt_connect]
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 fs/file_table.c       |  1 +
 net/sunrpc/xprt.c     |  5 +----
 net/sunrpc/xprtsock.c | 16 +++++++++++++---
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 30d55c9a1744..70e8fb68a171 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -375,6 +375,7 @@ void __fput_sync(struct file *file)
 }
 
 EXPORT_SYMBOL(fput);
+EXPORT_SYMBOL(__fput_sync);
 
 void __init files_init(void)
 {
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 68d08dcba018..94ae95c57f78 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -868,10 +868,7 @@ void xprt_connect(struct rpc_task *task)
 	if (!xprt_lock_write(xprt, task))
 		return;
 
-	if (test_and_clear_bit(XPRT_CLOSE_WAIT, &xprt->state))
-		xprt->ops->close(xprt);
-
-	if (!xprt_connected(xprt)) {
+	if (!xprt_connected(xprt) && !test_bit(XPRT_CLOSE_WAIT, &xprt->state)) {
 		task->tk_rqstp->rq_connect_cookie = xprt->connect_cookie;
 		rpc_sleep_on_timeout(&xprt->pending, task, NULL,
 				xprt_request_timeout(task->tk_rqstp));
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 29e9c54a89d3..81f0e03b71b6 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -989,7 +989,7 @@ static int xs_local_send_request(struct rpc_rqst *req)
 
 	/* Close the stream if the previous transmission was incomplete */
 	if (xs_send_request_was_aborted(transport, req)) {
-		xs_close(xprt);
+		xprt_force_disconnect(xprt);
 		return -ENOTCONN;
 	}
 
@@ -1027,7 +1027,7 @@ static int xs_local_send_request(struct rpc_rqst *req)
 			-status);
 		/* fall through */
 	case -EPIPE:
-		xs_close(xprt);
+		xprt_force_disconnect(xprt);
 		status = -ENOTCONN;
 	}
 
@@ -1303,6 +1303,16 @@ static void xs_reset_transport(struct sock_xprt *transport)
 
 	if (sk == NULL)
 		return;
+	/*
+	 * Make sure we're calling this in a context from which it is safe
+	 * to call __fput_sync(). In practice that means rpciod and the
+	 * system workqueue.
+	 */
+	if (!(current->flags & PF_WQ_WORKER)) {
+		WARN_ON_ONCE(1);
+		set_bit(XPRT_CLOSE_WAIT, &xprt->state);
+		return;
+	}
 
 	if (atomic_read(&transport->xprt.swapper))
 		sk_clear_memalloc(sk);
@@ -1326,7 +1336,7 @@ static void xs_reset_transport(struct sock_xprt *transport)
 	mutex_unlock(&transport->recv_mutex);
 
 	trace_rpc_socket_close(xprt, sock);
-	fput(filp);
+	__fput_sync(filp);
 
 	xprt_disconnect_done(xprt);
 }
-- 
2.36.1.124.g0e6072fb45-goog

Re: [PATCH 5.4 0/4] Request to cherry-pick f00432063db1 to 5.4.y

[Greg KH]

On Wed, May 18, 2022 at 06:40:07PM +0000, Meena Shanmugam wrote:
> The commit f00432063db1a0db484e85193eccc6845435b80e (SUNRPC:
> Ensure we flush any closed sockets before xs_xprt_free()) upstream fixes
> CVE-2022-28893, hence good candidate for stable trees.
> The above commit depends on 3be232f11a3c (SUNRPC: Prevent immediate
> close+reconnect)  and  89f42494f92f (SUNRPC: Don't call connect() more than
> once on a TCP socket). Commit 3be232f11a3c depends on commit
> e26d9972720e (SUNRPC: Clean up scheduling of autoclose).
> 
> Commits e26d9972720e, 3be232f11a3c apply cleanly on 5.4
> kernel. commit 89f42494f92f and f00432063db1 didn't apply cleanly.
> This patch series includes all the commits required for back porting
> f00432063db1.
> 
> 
> Trond Myklebust (4):
>   SUNRPC: Clean up scheduling of autoclose
>   SUNRPC: Prevent immediate close+reconnect
>   SUNRPC: Don't call connect() more than once on a TCP socket
>   SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()
> 
>  fs/file_table.c                 |  1 +
>  include/linux/sunrpc/xprtsock.h |  1 +
>  net/sunrpc/xprt.c               | 34 ++++++++++++++++---------------
>  net/sunrpc/xprtsock.c           | 36 ++++++++++++++++++++++-----------
>  4 files changed, 44 insertions(+), 28 deletions(-)
> 
> -- 
> 2.36.1.124.g0e6072fb45-goog
> 

All now queued up, thanks.

greg k-h