CIFS: Fix a possible memory corruption during reconnect

commit 53e0e11efe9289535b060a51d4cf37c25e0d0f2b upstream.

We can not unlock/lock cifs_tcp_ses_lock while walking through ses
and tcon lists because it can corrupt list iterator pointers and
a tcon structure can be released if we don't hold an extra reference.
Fix it by moving a reconnect process to a separate delayed work
and acquiring a reference to every tcon that needs to be reconnected.
Also do not send an echo request on newly established connections.

Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Willy Tarreau <w@1wt.eu>

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index f74dfa89c4c46db569bcb4e157b4779701e3efd0..28708bbb3457e1a79c17450a6148fe5d4e7ac02d 100644 (file)
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -576,6 +576,8 @@ struct TCP_Server_Info {
 #ifdef CONFIG_CIFS_SMB2
        unsigned int    max_read;
        unsigned int    max_write;
+       struct delayed_work reconnect; /* reconnect workqueue job */
+       struct mutex reconnect_mutex; /* prevent simultaneous reconnects */
 #endif /* CONFIG_CIFS_SMB2 */
 };
 
@@ -750,6 +752,7 @@ cap_unix(struct cifs_ses *ses)
 struct cifs_tcon {
        struct list_head tcon_list;
        int tc_count;
+       struct list_head rlist; /* reconnect list */
        struct list_head openFileList;
        struct cifs_ses *ses;   /* pointer to session associated with */
        char treeName[MAX_TREE_SIZE + 1]; /* UNC name of resource in ASCII */

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index dda188a94332bfbf569121680d3207360e1acc3a..1194a8ba1034de1d6769b9e876ef20a5fea63ce5 100644 (file)
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -194,6 +194,9 @@ extern void cifs_add_pending_open_locked(struct cifs_fid *fid,
                                         struct tcon_link *tlink,
                                         struct cifs_pending_open *open);
 extern void cifs_del_pending_open(struct cifs_pending_open *open);
+extern void cifs_put_tcp_session(struct TCP_Server_Info *server,
+                                int from_reconnect);
+extern void cifs_put_tcon(struct cifs_tcon *tcon);
 
 #if IS_ENABLED(CONFIG_CIFS_DFS_UPCALL)
 extern void cifs_dfs_release_automount_timer(void);

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7c33afd7d5d3d31ad2169c16d5fa17644540cd8a..0808b0852e795a9ea968e74e7e2856938018e04d 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -52,6 +52,9 @@
 #include "nterr.h"
 #include "rfc1002pdu.h"
 #include "fscache.h"
+#ifdef CONFIG_CIFS_SMB2
+#include "smb2proto.h"
+#endif
 
 #define CIFS_PORT 445
 #define RFC1001_PORT 139
@@ -2070,8 +2073,8 @@ cifs_find_tcp_session(struct smb_vol *vol)
        return NULL;
 }
 
-static void
-cifs_put_tcp_session(struct TCP_Server_Info *server)
+void
+cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
 {
        struct task_struct *task;
 
@@ -2088,6 +2091,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
 
        cancel_delayed_work_sync(&server->echo);
 
+#ifdef CONFIG_CIFS_SMB2
+       if (from_reconnect)
+               /*
+                * Avoid deadlock here: reconnect work calls
+                * cifs_put_tcp_session() at its end. Need to be sure
+                * that reconnect work does nothing with server pointer after
+                * that step.
+                */
+               cancel_delayed_work(&server->reconnect);
+       else
+               cancel_delayed_work_sync(&server->reconnect);
+#endif
+
        spin_lock(&GlobalMid_Lock);
        server->tcpStatus = CifsExiting;
        spin_unlock(&GlobalMid_Lock);
@@ -2158,6 +2174,10 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
        INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
        INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
        INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);
+#ifdef CONFIG_CIFS_SMB2
+       INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server);
+       mutex_init(&tcp_ses->reconnect_mutex);
+#endif
        memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr,
               sizeof(tcp_ses->srcaddr));
        memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
@@ -2288,7 +2308,7 @@ cifs_put_smb_ses(struct cifs_ses *ses)
                _free_xid(xid);
        }
        sesInfoFree(ses);
-       cifs_put_tcp_session(server);
+       cifs_put_tcp_session(server, 0);
 }
 
 #ifdef CONFIG_KEYS
@@ -2461,7 +2481,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
                mutex_unlock(&ses->session_mutex);
 
                /* existing SMB ses has a server reference already */
-               cifs_put_tcp_session(server);
+               cifs_put_tcp_session(server, 0);
                free_xid(xid);
                return ses;
        }
@@ -2550,7 +2570,7 @@ cifs_find_tcon(struct cifs_ses *ses, const char *unc)
        return NULL;
 }
 
-static void
+void
 cifs_put_tcon(struct cifs_tcon *tcon)
 {
        unsigned int xid;
@@ -3599,7 +3619,7 @@ mount_fail_check:
                else if (ses)
                        cifs_put_smb_ses(ses);
                else
-                       cifs_put_tcp_session(server);
+                       cifs_put_tcp_session(server, 0);
                bdi_destroy(&cifs_sb->bdi);
        }
 
@@ -3932,7 +3952,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
        ses = cifs_get_smb_ses(master_tcon->ses->server, vol_info);
        if (IS_ERR(ses)) {
                tcon = (struct cifs_tcon *)ses;
-               cifs_put_tcp_session(master_tcon->ses->server);
+               cifs_put_tcp_session(master_tcon->ses->server, 0);
                goto out;
        }
 

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 9dd8c968d94edcf8d25ccefe586371a169272799..133067cfb3f9b1d37a5775c8016bc1e4f8ea2c83 100644 (file)
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1239,6 +1239,54 @@ smb2_echo_callback(struct mid_q_entry *mid)
        add_credits(server, credits_received, CIFS_ECHO_OP);
 }
 
+void smb2_reconnect_server(struct work_struct *work)
+{
+       struct TCP_Server_Info *server = container_of(work,
+                                       struct TCP_Server_Info, reconnect.work);
+       struct cifs_ses *ses;
+       struct cifs_tcon *tcon, *tcon2;
+       struct list_head tmp_list;
+       int tcon_exist = false;
+
+       /* Prevent simultaneous reconnects that can corrupt tcon->rlist list */
+       mutex_lock(&server->reconnect_mutex);
+
+       INIT_LIST_HEAD(&tmp_list);
+       cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
+
+       spin_lock(&cifs_tcp_ses_lock);
+       list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+               list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
+                       if (tcon->need_reconnect) {
+                               tcon->tc_count++;
+                               list_add_tail(&tcon->rlist, &tmp_list);
+                               tcon_exist = true;
+                       }
+               }
+       }
+       /*
+        * Get the reference to server struct to be sure that the last call of
+        * cifs_put_tcon() in the loop below won't release the server pointer.
+        */
+       if (tcon_exist)
+               server->srv_count++;
+
+       spin_unlock(&cifs_tcp_ses_lock);
+
+       list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
+               smb2_reconnect(SMB2_ECHO, tcon);
+               list_del_init(&tcon->rlist);
+               cifs_put_tcon(tcon);
+       }
+
+       cifs_dbg(FYI, "Reconnecting tcons finished\n");
+       mutex_unlock(&server->reconnect_mutex);
+
+       /* now we can safely release srv struct */
+       if (tcon_exist)
+               cifs_put_tcp_session(server, 1);
+}
+
 int
 SMB2_echo(struct TCP_Server_Info *server)
 {
@@ -1251,32 +1299,11 @@ SMB2_echo(struct TCP_Server_Info *server)
        cifs_dbg(FYI, "In echo request\n");
 
        if (server->tcpStatus == CifsNeedNegotiate) {
-               struct list_head *tmp, *tmp2;
-               struct cifs_ses *ses;
-               struct cifs_tcon *tcon;
-
-               cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
-               spin_lock(&cifs_tcp_ses_lock);
-               list_for_each(tmp, &server->smb_ses_list) {
-                       ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
-                       list_for_each(tmp2, &ses->tcon_list) {
-                               tcon = list_entry(tmp2, struct cifs_tcon,
-                                                 tcon_list);
-                               /* add check for persistent handle reconnect */
-                               if (tcon && tcon->need_reconnect) {
-                                       spin_unlock(&cifs_tcp_ses_lock);
-                                       rc = smb2_reconnect(SMB2_ECHO, tcon);
-                                       spin_lock(&cifs_tcp_ses_lock);
-                               }
-                       }
-               }
-               spin_unlock(&cifs_tcp_ses_lock);
+               /* No need to send echo on newly established connections */
+               queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
+               return rc;
        }
 
-       /* if no session, renegotiate failed above */
-       if (server->tcpStatus == CifsNeedNegotiate)
-               return -EIO;
-
        rc = small_smb2_init(SMB2_ECHO, NULL, (void **)&req);
        if (rc)
                return rc;

diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 2aa3535e38ced630c3c6e7716ff3540769e46873..d0cd166ac887304be623238db440c5316e66b240 100644 (file)
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -93,6 +93,7 @@ extern void smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock);
 extern int smb2_unlock_range(struct cifsFileInfo *cfile,
                             struct file_lock *flock, const unsigned int xid);
 extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
+extern void smb2_reconnect_server(struct work_struct *work);
 
 /*
  * SMB2 Worker functions - most of protocol specific implementation details