Commit 96a988ff authored by Pavel Shilovsky's avatar Pavel Shilovsky

CIFS: Fix a possible double locking of mutex during reconnect

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.

Cc: Stable <stable@vger.kernel.org> # v4.9+
Signed-off-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
parent 53e0e11e
...@@ -926,6 +926,7 @@ struct cifs_tcon { ...@@ -926,6 +926,7 @@ struct cifs_tcon {
bool broken_posix_open; /* e.g. Samba server versions < 3.3.2, 3.2.9 */ 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 broken_sparse_sup; /* if server or share does not support sparse */
bool need_reconnect:1; /* connection reset, tid now invalid */ 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_resilient:1; /* use resilient instead of durable handles */
bool use_persistent:1; /* use persistent instead of durable handles */ bool use_persistent:1; /* use persistent instead of durable handles */
#ifdef CONFIG_CIFS_SMB2 #ifdef CONFIG_CIFS_SMB2
......
...@@ -777,6 +777,11 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon) ...@@ -777,6 +777,11 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon)
struct list_head *tmp1; struct list_head *tmp1;
struct list_head tmp_list; 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"); cifs_dbg(FYI, "Reopen persistent handles");
INIT_LIST_HEAD(&tmp_list); INIT_LIST_HEAD(&tmp_list);
...@@ -793,7 +798,8 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon) ...@@ -793,7 +798,8 @@ cifs_reopen_persistent_handles(struct cifs_tcon *tcon)
list_for_each_safe(tmp, tmp1, &tmp_list) { list_for_each_safe(tmp, tmp1, &tmp_list) {
open_file = list_entry(tmp, struct cifsFileInfo, rlist); 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); list_del_init(&open_file->rlist);
cifsFileInfo_put(open_file); cifsFileInfo_put(open_file);
} }
......
...@@ -250,16 +250,19 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon) ...@@ -250,16 +250,19 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
} }
cifs_mark_open_files_invalid(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); rc = SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nls_codepage);
mutex_unlock(&tcon->ses->session_mutex); mutex_unlock(&tcon->ses->session_mutex);
if (tcon->use_persistent)
cifs_reopen_persistent_handles(tcon);
cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc); cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
if (rc) if (rc)
goto out; goto out;
if (smb2_command != SMB2_INTERNAL_CMD)
queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
atomic_inc(&tconInfoReconnectCount); atomic_inc(&tconInfoReconnectCount);
out: out:
/* /*
...@@ -1990,7 +1993,7 @@ void smb2_reconnect_server(struct work_struct *work) ...@@ -1990,7 +1993,7 @@ void smb2_reconnect_server(struct work_struct *work)
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
list_for_each_entry(tcon, &ses->tcon_list, tcon_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++; tcon->tc_count++;
list_add_tail(&tcon->rlist, &tmp_list); list_add_tail(&tcon->rlist, &tmp_list);
tcon_exist = true; tcon_exist = true;
...@@ -2007,7 +2010,8 @@ void smb2_reconnect_server(struct work_struct *work) ...@@ -2007,7 +2010,8 @@ void smb2_reconnect_server(struct work_struct *work)
spin_unlock(&cifs_tcp_ses_lock); spin_unlock(&cifs_tcp_ses_lock);
list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) { 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); list_del_init(&tcon->rlist);
cifs_put_tcon(tcon); cifs_put_tcon(tcon);
} }
......
...@@ -80,6 +80,8 @@ ...@@ -80,6 +80,8 @@
#define SMB2_SET_INFO cpu_to_le16(SMB2_SET_INFO_HE) #define SMB2_SET_INFO cpu_to_le16(SMB2_SET_INFO_HE)
#define SMB2_OPLOCK_BREAK cpu_to_le16(SMB2_OPLOCK_BREAK_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 #define NUMBER_OF_SMB2_COMMANDS 0x0013
/* BB FIXME - analyze following length BB */ /* BB FIXME - analyze following length BB */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment