Commit bda487ac authored by Shyam Prasad N's avatar Shyam Prasad N Committed by Steve French
Browse files

cifs: avoid race during socket reconnect between send and recv 



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: default avatarShyam Prasad N <sprasad@microsoft.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 73f9bfbe

fs/cifs/connect.c

+3 −11
Original line number Original line Diff line number Diff line
@@ -172,12 +172,11 @@ static void 
cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,

cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,

				      bool mark_smb_session)

				      bool mark_smb_session)

{

{

	unsigned int num_sessions = 0;

	struct TCP_Server_Info *pserver;

	struct cifs_ses *ses;

	struct cifs_ses *ses;

	struct cifs_tcon *tcon;

	struct cifs_tcon *tcon;

	struct mid_q_entry *mid, *nmid;

	struct mid_q_entry *mid, *nmid;

	struct list_head retry_list;

	struct list_head retry_list;

	struct TCP_Server_Info *pserver;





	server->maxBuf = 0;

	server->maxBuf = 0;

	server->max_read = 0;

	server->max_read = 0;
@@ -199,9 +198,6 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, 
		if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))

		if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))

			goto next_session;

			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 all channels need reconnect, then tcon needs reconnect */
@@ -209,7 +205,6 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, 
			goto next_session;

			goto next_session;





		ses->status = CifsNeedReconnect;

		ses->status = CifsNeedReconnect;

		num_sessions++;





		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {

		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {

			tcon->need_reconnect = true;

			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);

	spin_unlock(&cifs_tcp_ses_lock);





	if (num_sessions == 0)

		return;

	/*

	/*

	 * before reconnecting the tcp session, mark the smb session (uid)

	 * before reconnecting the tcp session, mark the smb session (uid)

	 * and the tid bad so they are not used until reconnected

	 * 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__);

		 __func__);

	/* do not want to be sending data on a socket we are freeing */

	/* 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);

	mutex_lock(&server->srv_mutex);

	if (server->ssocket) {

	if (server->ssocket) {

		cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state,

		cifs_dbg(FYI, "State: 0x%x Flags: 0x%lx\n", server->ssocket->state,

fs/cifs/smb2pdu.c

+1 −2
Original line number Original line Diff line number Diff line
@@ -276,7 +276,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon, 
		if (tcon->need_reconnect)

		if (tcon->need_reconnect)

			goto skip_sess_setup;

			goto skip_sess_setup;





		rc = -EHOSTDOWN;

		goto out;

		goto out;

	}

	}

	spin_unlock(&ses->chan_lock);

	spin_unlock(&ses->chan_lock);
@@ -3858,7 +3857,7 @@ SMB2_echo(struct TCP_Server_Info *server) 
				 .rq_nvec = 1 };

				 .rq_nvec = 1 };

	unsigned int total_len;

	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);

	spin_lock(&cifs_tcp_ses_lock);

	if (server->tcpStatus == CifsNeedNegotiate) {

	if (server->tcpStatus == CifsNeedNegotiate) {

fs/cifs/transport.c

+2 −11
Original line number Original line Diff line number Diff line
@@ -1057,19 +1057,10 @@ struct TCP_Server_Info *cifs_pick_channel(struct cifs_ses *ses) 
	if (!ses)

	if (!ses)

		return NULL;

		return NULL;





	spin_lock(&ses->chan_lock);

	/* round robin */

	/* round robin */

pick_another:

	if (ses->chan_count > 1 &&

	    !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {

	index = (uint)atomic_inc_return(&ses->chan_seq);

	index = (uint)atomic_inc_return(&ses->chan_seq);

	index %= ses->chan_count;

	index %= ses->chan_count;





		if (CIFS_CHAN_NEEDS_RECONNECT(ses, index))

			goto pick_another;

	}

	spin_unlock(&ses->chan_lock);



	return ses->chans[index].server;

	return ses->chans[index].server;

}

}