All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Shilovsky <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
To: <linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: [PATCH 5/5] CIFS: Fix a possible double locking of mutex during reconnect
Date: Tue, 29 Nov 2016 16:56:56 -0800	[thread overview]
Message-ID: <1480467416-13636-6-git-send-email-pshilov@microsoft.com> (raw)
In-Reply-To: <1480467416-13636-1-git-send-email-pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>

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.

Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.9+
Signed-off-by: Pavel Shilovsky <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
---
 fs/cifs/cifsglob.h |  1 +
 fs/cifs/file.c     |  8 +++++++-
 fs/cifs/smb2pdu.c  | 14 +++++++++-----
 fs/cifs/smb2pdu.h  |  2 ++
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 52368a6..6458a17 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -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
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 7f5f617..18a1e1d 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -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);
 	}
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 8fecf4b..7b13f7b 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -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)
+		mod_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);
 	}
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index fd3709e..dc0d141 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -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 */
-- 
2.7.4

      parent reply	other threads:[~2016-11-30  0:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-30  0:56 [PATCH 0/5] Stable fixes Pavel Shilovsky
     [not found] ` <1480467416-13636-1-git-send-email-pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
2016-11-30  0:56   ` [PATCH 1/5] CIFS: Decrease verbosity of ioctl call Pavel Shilovsky
     [not found]     ` <1480467416-13636-2-git-send-email-pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
2016-12-05  6:30       ` Sachin Prabhu
2016-11-30  0:56   ` [PATCH 2/5] CIFS: Fix missing nls unload in smb2_reconnect() Pavel Shilovsky
     [not found]     ` <1480467416-13636-3-git-send-email-pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>
2016-12-05  6:48       ` Sachin Prabhu
     [not found]         ` <1480920506.3464.3.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-12-05  8:02           ` Steve French
     [not found]             ` <CAH2r5msBN2Gh==g5BOC5Woe0WaNvXxug38tfvG4cGUvUo7MovA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-12-05 19:03               ` Pavel Shilovsky
     [not found]                 ` <CAKywueROW-494MfgyAo_8OHc0UOZxX510dGjk9DEDG-Fx1K-hQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-12-06  5:00                   ` Steve French
2016-11-30  0:56   ` [PATCH 3/5] CIFS: Fix a possible memory corruption in push locks Pavel Shilovsky
2016-11-30  0:56   ` [PATCH 4/5] CIFS: Fix a possible memory corruption during reconnect Pavel Shilovsky
2016-11-30  0:56   ` Pavel Shilovsky [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480467416-13636-6-git-send-email-pshilov@microsoft.com \
    --to=pshilov-0li6otcxbfhby3ivrkzq2a@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.