| From f00432063db1a0db484e85193eccc6845435b80e Mon Sep 17 00:00:00 2001 |
| From: Trond Myklebust <trond.myklebust@hammerspace.com> |
| Date: Sun, 3 Apr 2022 15:58:11 -0400 |
| Subject: SUNRPC: Ensure we flush any closed sockets before xs_xprt_free() |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/file_table.c | 1 + |
| include/trace/events/sunrpc.h | 1 - |
| net/sunrpc/xprt.c | 7 +------ |
| net/sunrpc/xprtsock.c | 16 +++++++++++++--- |
| 4 files changed, 15 insertions(+), 10 deletions(-) |
| |
| --- 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) |
| { |
| --- a/include/trace/events/sunrpc.h |
| +++ b/include/trace/events/sunrpc.h |
| @@ -993,7 +993,6 @@ DEFINE_RPC_XPRT_LIFETIME_EVENT(connect); |
| DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_auto); |
| DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_done); |
| DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_force); |
| -DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_cleanup); |
| DEFINE_RPC_XPRT_LIFETIME_EVENT(destroy); |
| |
| DECLARE_EVENT_CLASS(rpc_xprt_event, |
| --- a/net/sunrpc/xprt.c |
| +++ b/net/sunrpc/xprt.c |
| @@ -929,12 +929,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)) { |
| - trace_xprt_disconnect_cleanup(xprt); |
| - 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)); |
| --- a/net/sunrpc/xprtsock.c |
| +++ b/net/sunrpc/xprtsock.c |
| @@ -880,7 +880,7 @@ static int xs_local_send_request(struct |
| |
| /* 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; |
| } |
| |
| @@ -918,7 +918,7 @@ static int xs_local_send_request(struct |
| -status); |
| fallthrough; |
| case -EPIPE: |
| - xs_close(xprt); |
| + xprt_force_disconnect(xprt); |
| status = -ENOTCONN; |
| } |
| |
| @@ -1203,6 +1203,16 @@ static void xs_reset_transport(struct so |
| |
| 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); |
| @@ -1226,7 +1236,7 @@ static void xs_reset_transport(struct so |
| mutex_unlock(&transport->recv_mutex); |
| |
| trace_rpc_socket_close(xprt, sock); |
| - fput(filp); |
| + __fput_sync(filp); |
| |
| xprt_disconnect_done(xprt); |
| } |