Skip to content

Commit

Permalink
cifs: avoid race during socket reconnect between send and recv
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Shyam Prasad N authored and Steve French committed Jan 8, 2022
1 parent 73f9bfb commit bda487a
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 24 deletions.
14 changes: 3 additions & 11 deletions fs/cifs/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions fs/cifs/smb2pdu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 2 additions & 11 deletions fs/cifs/transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit bda487a

Please sign in to comment.