cifs: avoid race conditions with parallel reconnects

When multiple processes/channels do reconnects in parallel
we used to return success immediately
negotiate/session-setup/tree-connect, causing race conditions
between processes that enter the function in parallel.
This caused several errors related to session not found to
show up during parallel reconnects.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Reviewed-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index f42cc70773122a6a712e39aca6e2eb6cfdda90b1..c3162ef9c9e93a028a0604222fc1bb13cc40153b 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -212,31 +212,42 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
                        cifs_chan_update_iface(ses, server);
 
                spin_lock(&ses->chan_lock);
-               if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))
-                       goto next_session;
+               if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server)) {
+                       spin_unlock(&ses->chan_lock);
+                       continue;
+               }
 
                if (mark_smb_session)
                        CIFS_SET_ALL_CHANS_NEED_RECONNECT(ses);
                else
                        cifs_chan_set_need_reconnect(ses, server);
 
+               cifs_dbg(FYI, "%s: channel connect bitmap: 0x%lx\n",
+                        __func__, ses->chans_need_reconnect);
+
                /* If all channels need reconnect, then tcon needs reconnect */
-               if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
-                       goto next_session;
+               if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
+                       spin_unlock(&ses->chan_lock);
+                       continue;
+               }
+               spin_unlock(&ses->chan_lock);
 
+               spin_lock(&ses->ses_lock);
                ses->ses_status = SES_NEED_RECON;
+               spin_unlock(&ses->ses_lock);
 
                list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
                        tcon->need_reconnect = true;
+                       spin_lock(&tcon->tc_lock);
                        tcon->status = TID_NEED_RECON;
+                       spin_unlock(&tcon->tc_lock);
                }
                if (ses->tcon_ipc) {
                        ses->tcon_ipc->need_reconnect = true;
+                       spin_lock(&ses->tcon_ipc->tc_lock);
                        ses->tcon_ipc->status = TID_NEED_RECON;
+                       spin_unlock(&ses->tcon_ipc->tc_lock);
                }
-
-next_session:
-               spin_unlock(&ses->chan_lock);
        }
        spin_unlock(&cifs_tcp_ses_lock);
 }
@@ -3653,11 +3664,19 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
 
        /* only send once per connect */
        spin_lock(&server->srv_lock);
-       if (!server->ops->need_neg(server) ||
+       if (server->tcpStatus != CifsGood &&
+           server->tcpStatus != CifsNew &&
            server->tcpStatus != CifsNeedNegotiate) {
+               spin_unlock(&server->srv_lock);
+               return -EHOSTDOWN;
+       }
+
+       if (!server->ops->need_neg(server) &&
+           server->tcpStatus == CifsGood) {
                spin_unlock(&server->srv_lock);
                return 0;
        }
+
        server->tcpStatus = CifsInNegotiate;
        spin_unlock(&server->srv_lock);
 
@@ -3691,23 +3710,28 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
        bool is_binding = false;
 
        spin_lock(&ses->ses_lock);
+       cifs_dbg(FYI, "%s: channel connect bitmap: 0x%lx\n",
+                __func__, ses->chans_need_reconnect);
+
        if (ses->ses_status != SES_GOOD &&
            ses->ses_status != SES_NEW &&
            ses->ses_status != SES_NEED_RECON) {
                spin_unlock(&ses->ses_lock);
-               return 0;
+               return -EHOSTDOWN;
        }
 
        /* only send once per connect */
        spin_lock(&ses->chan_lock);
-       if (CIFS_ALL_CHANS_GOOD(ses) ||
-           cifs_chan_in_reconnect(ses, server)) {
+       if (CIFS_ALL_CHANS_GOOD(ses)) {
+               if (ses->ses_status == SES_NEED_RECON)
+                       ses->ses_status = SES_GOOD;
                spin_unlock(&ses->chan_lock);
                spin_unlock(&ses->ses_lock);
                return 0;
        }
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
+
        cifs_chan_set_in_reconnect(ses, server);
+       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
        spin_unlock(&ses->chan_lock);
 
        if (!is_binding)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index caeb92a69b5f4a38101616389dccbdb64330e182..a9fb95b7ef8292196970bdf1d4471338c85b98f2 100644 (file)
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -203,6 +203,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
        }
        spin_unlock(&server->srv_lock);
 
+again:
        rc = cifs_wait_for_server_reconnect(server, tcon->retry);
        if (rc)
                return rc;
@@ -221,6 +222,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 
        nls_codepage = load_nls_default();
 
+       mutex_lock(&ses->session_mutex);
        /*
         * Recheck after acquire mutex. If another thread is negotiating
         * and the server never sends an answer the socket will be closed
@@ -229,6 +231,11 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
        spin_lock(&server->srv_lock);
        if (server->tcpStatus == CifsNeedReconnect) {
                spin_unlock(&server->srv_lock);
+               mutex_unlock(&ses->session_mutex);
+
+               if (tcon->retry)
+                       goto again;
+
                rc = -EHOSTDOWN;
                goto out;
        }
@@ -238,19 +245,22 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
         * need to prevent multiple threads trying to simultaneously
         * reconnect the same SMB session
         */
+       spin_lock(&ses->ses_lock);
        spin_lock(&ses->chan_lock);
-       if (!cifs_chan_needs_reconnect(ses, server)) {
+       if (!cifs_chan_needs_reconnect(ses, server) &&
+           ses->ses_status == SES_GOOD) {
                spin_unlock(&ses->chan_lock);
-
+               spin_unlock(&ses->ses_lock);
                /* this means that we only need to tree connect */
                if (tcon->need_reconnect)
                        goto skip_sess_setup;
 
+               mutex_unlock(&ses->session_mutex);
                goto out;
        }
        spin_unlock(&ses->chan_lock);
+       spin_unlock(&ses->ses_lock);
 
-       mutex_lock(&ses->session_mutex);
        rc = cifs_negotiate_protocol(0, ses, server);
        if (!rc) {
                rc = cifs_setup_session(0, ses, server, nls_codepage);
@@ -266,10 +276,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
                mutex_unlock(&ses->session_mutex);
                goto out;
        }
-       mutex_unlock(&ses->session_mutex);
 
 skip_sess_setup:
-       mutex_lock(&ses->session_mutex);
        if (!tcon->need_reconnect) {
                mutex_unlock(&ses->session_mutex);
                goto out;
@@ -284,7 +292,7 @@ skip_sess_setup:
        cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
        if (rc) {
                /* If sess reconnected but tcon didn't, something strange ... */
-               pr_warn_once("reconnect tcon failed rc = %d\n", rc);
+               cifs_dbg(VFS, "reconnect tcon failed rc = %d\n", rc);
                goto out;
        }
 
@@ -1256,9 +1264,9 @@ SMB2_sess_alloc_buffer(struct SMB2_sess_data *sess_data)
        if (rc)
                return rc;
 
-       spin_lock(&ses->chan_lock);
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
-       spin_unlock(&ses->chan_lock);
+       spin_lock(&ses->ses_lock);
+       is_binding = (ses->ses_status == SES_GOOD);
+       spin_unlock(&ses->ses_lock);
 
        if (is_binding) {
                req->hdr.SessionId = cpu_to_le64(ses->Suid);
@@ -1416,9 +1424,9 @@ SMB2_auth_kerberos(struct SMB2_sess_data *sess_data)
                goto out_put_spnego_key;
        }
 
-       spin_lock(&ses->chan_lock);
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
-       spin_unlock(&ses->chan_lock);
+       spin_lock(&ses->ses_lock);
+       is_binding = (ses->ses_status == SES_GOOD);
+       spin_unlock(&ses->ses_lock);
 
        /* keep session key if binding */
        if (!is_binding) {
@@ -1542,9 +1550,9 @@ SMB2_sess_auth_rawntlmssp_negotiate(struct SMB2_sess_data *sess_data)
 
        cifs_dbg(FYI, "rawntlmssp session setup challenge phase\n");
 
-       spin_lock(&ses->chan_lock);
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
-       spin_unlock(&ses->chan_lock);
+       spin_lock(&ses->ses_lock);
+       is_binding = (ses->ses_status == SES_GOOD);
+       spin_unlock(&ses->ses_lock);
 
        /* keep existing ses id and flags if binding */
        if (!is_binding) {
@@ -1610,9 +1618,9 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
 
        rsp = (struct smb2_sess_setup_rsp *)sess_data->iov[0].iov_base;
 
-       spin_lock(&ses->chan_lock);
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
-       spin_unlock(&ses->chan_lock);
+       spin_lock(&ses->ses_lock);
+       is_binding = (ses->ses_status == SES_GOOD);
+       spin_unlock(&ses->ses_lock);
 
        /* keep existing ses id and flags if binding */
        if (!is_binding) {

diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index d827b7547ffadaeb97e6609b821dbd9e63f159dc..790acf65a0926cbe3316d591a3bc7a1b97120974 100644 (file)
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -81,6 +81,7 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
        struct cifs_ses *ses = NULL;
        int i;
        int rc = 0;
+       bool is_binding = false;
 
        spin_lock(&cifs_tcp_ses_lock);
 
@@ -97,9 +98,12 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
        goto out;
 
 found:
+       spin_lock(&ses->ses_lock);
        spin_lock(&ses->chan_lock);
-       if (cifs_chan_needs_reconnect(ses, server) &&
-           !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
+
+       is_binding = (cifs_chan_needs_reconnect(ses, server) &&
+                     ses->ses_status == SES_GOOD);
+       if (is_binding) {
                /*
                 * If we are in the process of binding a new channel
                 * to an existing session, use the master connection
@@ -107,6 +111,7 @@ found:
                 */
                memcpy(key, ses->smb3signingkey, SMB3_SIGN_KEY_SIZE);
                spin_unlock(&ses->chan_lock);
+               spin_unlock(&ses->ses_lock);
                goto out;
        }
 
@@ -119,10 +124,12 @@ found:
                if (chan->server == server) {
                        memcpy(key, chan->signkey, SMB3_SIGN_KEY_SIZE);
                        spin_unlock(&ses->chan_lock);
+                       spin_unlock(&ses->ses_lock);
                        goto out;
                }
        }
        spin_unlock(&ses->chan_lock);
+       spin_unlock(&ses->ses_lock);
 
        cifs_dbg(VFS,
                 "%s: Could not find channel signing key for session 0x%llx\n",
@@ -392,11 +399,15 @@ generate_smb3signingkey(struct cifs_ses *ses,
        bool is_binding = false;
        int chan_index = 0;
 
+       spin_lock(&ses->ses_lock);
        spin_lock(&ses->chan_lock);
-       is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
+       is_binding = (cifs_chan_needs_reconnect(ses, server) &&
+                     ses->ses_status == SES_GOOD);
+
        chan_index = cifs_ses_get_chan_index(ses, server);
        /* TODO: introduce ref counting for channels when the can be freed */
        spin_unlock(&ses->chan_lock);
+       spin_unlock(&ses->ses_lock);
 
        /*
         * All channels use the same encryption/decryption keys but