CIFS: Fix a possible double locking of mutex during reconnect
authorPavel Shilovsky <pshilov@microsoft.com>
Tue, 29 Nov 2016 19:31:23 +0000 (11:31 -0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 6 Jan 2017 09:40:16 +0000 (10:40 +0100)
commit 96a988ffeb90dba33a71c3826086fe67c897a183 upstream.

With the current code it is possible to lock a mutex twice when
a subsequent reconnects are triggered. On the 1st reconnect we
reconnect sessions and tcons and then persistent file handles.
If the 2nd reconnect happens during the reconnecting of persistent
file handles then the following sequence of calls is observed:

cifs_reopen_file -> SMB2_open -> small_smb2_init -> smb2_reconnect
-> cifs_reopen_persistent_file_handles -> cifs_reopen_file (again!).

So, we are trying to acquire the same cfile->fh_mutex twice which
is wrong. Fix this by moving reconnecting of persistent handles to
the delayed work (smb2_reconnect_server) and submitting this work
every time we reconnect tcon in SMB2 commands handling codepath.

This can also lead to corruption of a temporary file list in
cifs_reopen_persistent_file_handles() because we can recursively
call this function twice.

Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/cifs/cifsglob.h
fs/cifs/file.c
fs/cifs/smb2pdu.c
fs/cifs/smb2pdu.h

index 64f5d8754fb542d936261e791985f4ec73d23679..203287f86525459bbd233b001b3b333437953dcf 100644 (file)
@@ -925,6 +925,7 @@ struct cifs_tcon {
        bool broken_posix_open; /* e.g. Samba server versions < 3.3.2, 3.2.9 */
        bool broken_sparse_sup; /* if server or share does not support sparse */
        bool need_reconnect:1; /* connection reset, tid now invalid */
+       bool need_reopen_files:1; /* need to reopen tcon file handles */
        bool use_resilient:1; /* use resilient instead of durable handles */
        bool use_persistent:1; /* use persistent instead of durable handles */
 #ifdef CONFIG_CIFS_SMB2
index 7f5f6176c6f15caff307e078320122141c119ab9..18a1e1d6671fc0825b2f265280a96e065e6c6aae 100644 (file)
@@ -777,6 +777,11 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon)
        struct list_head *tmp1;
        struct list_head tmp_list;
 
+       if (!tcon->use_persistent || !tcon->need_reopen_files)
+               return;
+
+       tcon->need_reopen_files = false;
+
        cifs_dbg(FYI, "Reopen persistent handles");
        INIT_LIST_HEAD(&tmp_list);
 
@@ -793,7 +798,8 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon)
 
        list_for_each_safe(tmp, tmp1, &tmp_list) {
                open_file = list_entry(tmp, struct cifsFileInfo, rlist);
-               cifs_reopen_file(open_file, false /* do not flush */);
+               if (cifs_reopen_file(open_file, false /* do not flush */))
+                       tcon->need_reopen_files = true;
                list_del_init(&open_file->rlist);
                cifsFileInfo_put(open_file);
        }
index 4ba3f68a1766c84518af2e428d4f02ed912b4231..87457227812c393fe1ee9e09bacd9f1c65d93f9e 100644 (file)
@@ -250,16 +250,19 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
        }
 
        cifs_mark_open_files_invalid(tcon);
+       if (tcon->use_persistent)
+               tcon->need_reopen_files = true;
 
        rc = SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nls_codepage);
        mutex_unlock(&tcon->ses->session_mutex);
 
-       if (tcon->use_persistent)
-               cifs_reopen_persistent_handles(tcon);
-
        cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
        if (rc)
                goto out;
+
+       if (smb2_command != SMB2_INTERNAL_CMD)
+               queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
        atomic_inc(&tconInfoReconnectCount);
 out:
        /*
@@ -1990,7 +1993,7 @@ void smb2_reconnect_server(struct work_struct *work)
        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) {
+                       if (tcon->need_reconnect || tcon->need_reopen_files) {
                                tcon->tc_count++;
                                list_add_tail(&tcon->rlist, &tmp_list);
                                tcon_exist = true;
@@ -2007,7 +2010,8 @@ void smb2_reconnect_server(struct work_struct *work)
        spin_unlock(&cifs_tcp_ses_lock);
 
        list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
-               smb2_reconnect(SMB2_ECHO, tcon);
+               if (!smb2_reconnect(SMB2_INTERNAL_CMD, tcon))
+                       cifs_reopen_persistent_handles(tcon);
                list_del_init(&tcon->rlist);
                cifs_put_tcon(tcon);
        }
index fd3709e8de33eddee8c66cca29cb6653921b020e..dc0d141f33e25730ea0242d2f946034e321cb1a4 100644 (file)
@@ -80,6 +80,8 @@
 #define SMB2_SET_INFO          cpu_to_le16(SMB2_SET_INFO_HE)
 #define SMB2_OPLOCK_BREAK      cpu_to_le16(SMB2_OPLOCK_BREAK_HE)
 
+#define SMB2_INTERNAL_CMD      cpu_to_le16(0xFFFF)
+
 #define NUMBER_OF_SMB2_COMMANDS        0x0013
 
 /* BB FIXME - analyze following length BB */