Blob Blame History Raw
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Mon, 25 Oct 2021 05:44:10 +0000
Subject: [PATCH] cifs: avoid race during socket reconnect between send and
 recv
Git-commit: bda487ac4bebf871255cc6f23e16f702cea0ca7c
References: bsc#1193629
Patch-mainline: v5.16

When a TCP connection gets reestablished by the sender in cifs_reconnect,
There is a chance for race condition with demultiplex thread waiting in
cifs_readv_from_socket on the old socket. It will now return -ECONNRESET.

This condition is handled by comparing socket pointer before and after
sock_recvmsg. If the socket pointer has changed, we should not call
cifs_reconnect again, but instead retry with new socket.

Also fixed another bug in my prev mchan commits.
We should always reestablish session (even if binding) on a channel
that needs reconnection.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Acked-by: Enzo Matsumiya <ematsumiya@suse.de>
---
 fs/cifs/connect.c   | 14 +++-----------
 fs/cifs/smb2pdu.c   |  3 +--
 fs/cifs/transport.c | 13 ++-----------
 3 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index a408187c7002..1dafaf7c4e5e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -172,12 +172,11 @@ static void
 cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
 				      bool mark_smb_session)
 {
-	unsigned int num_sessions = 0;
+	struct TCP_Server_Info *pserver;
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
 	struct mid_q_entry *mid, *nmid;
 	struct list_head retry_list;
-	struct TCP_Server_Info *pserver;
 
 	server->maxBuf = 0;
 	server->max_read = 0;
@@ -199,17 +198,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
 		if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))
 			goto next_session;
 
-		if (mark_smb_session)
-			CIFS_SET_ALL_CHANS_NEED_RECONNECT(ses);
-		else
-			cifs_chan_set_need_reconnect(ses, server);
+		cifs_chan_set_need_reconnect(ses, server);
 
 		/* If all channels need reconnect, then tcon needs reconnect */
 		if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
 			goto next_session;
 
 		ses->status = CifsNeedReconnect;
-		num_sessions++;
 
 		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
 			tcon->need_reconnect = true;
@@ -223,16 +218,13 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
 
-	if (num_sessions == 0)
-		return;
 	/*
 	 * before reconnecting the tcp session, mark the smb session (uid)
 	 * and the tid bad so they are not used until reconnected
 	 */
-	cifs_dbg(FYI, "%s: marking sessions and tcons for reconnect\n",
+	cifs_dbg(FYI, "%s: marking sessions and tcons for reconnect and tearing down socket\n",
 		 __func__);
 	/* do not want to be sending data on a socket we are freeing */
-	cifs_dbg(FYI, "%s: tearing down socket\n", __func__);
 	mutex_lock(&server->srv_mutex);
 	if (server->ssocket) {
 		cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2725e62470e4..8d471df69c59 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -276,7 +276,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 		if (tcon->need_reconnect)
 			goto skip_sess_setup;
 
-		rc = -EHOSTDOWN;
 		goto out;
 	}
 	spin_unlock(&ses->chan_lock);
@@ -3858,7 +3857,7 @@ SMB2_echo(struct TCP_Server_Info *server)
 				 .rq_nvec = 1 };
 	unsigned int total_len;
 
-	cifs_dbg(FYI, "In echo request\n");
+	cifs_dbg(FYI, "In echo request for conn_id %lld\n", server->conn_id);
 
 	spin_lock(&cifs_tcp_ses_lock);
 	if (server->tcpStatus == CifsNeedNegotiate) {
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 1c400ca26383..93f0e8c1ea23 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -1057,18 +1057,9 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses)
 	if (!ses)
 		return NULL;
 
-	spin_lock(&ses->chan_lock);
 	/* round robin */
-pick_another:
-	if (ses->chan_count > 1 &&
-	    !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
-		index = (uint)atomic_inc_return(&ses->chan_seq);
-		index %= ses->chan_count;
-
-		if (CIFS_CHAN_NEEDS_RECONNECT(ses, index))
-			goto pick_another;
-	}
-	spin_unlock(&ses->chan_lock);
+	index = (uint)atomic_inc_return(&ses->chan_seq);
+	index %= ses->chan_count;
 
 	return ses->chans[index].server;
 }
-- 
2.36.1