All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Slaby <jslaby@suse.cz>
To: stable@vger.kernel.org
Cc: Pavel Shilovsky <pshilov@microsoft.com>, Jiri Slaby <jslaby@suse.cz>
Subject: [patch added to 3.12-stable] CIFS: Fix a possible memory corruption during reconnect
Date: Fri, 13 Jan 2017 08:41:54 +0100	[thread overview]
Message-ID: <20170113074159.26811-28-jslaby@suse.cz> (raw)
In-Reply-To: <20170113074159.26811-1-jslaby@suse.cz>

From: Pavel Shilovsky <pshilov@microsoft.com>

This patch has been added to the 3.12 stable tree. If you have any
objections, please let us know.

===============

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>
---
 fs/cifs/cifsglob.h  |  3 +++
 fs/cifs/cifsproto.h |  3 +++
 fs/cifs/connect.c   | 34 +++++++++++++++++++-----
 fs/cifs/smb2pdu.c   | 75 ++++++++++++++++++++++++++++++++++++-----------------
 fs/cifs/smb2proto.h |  1 +
 5 files changed, 85 insertions(+), 31 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index fa30efe15ba2..4b87feaa507f 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -594,6 +594,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 */
 };
 
@@ -760,6 +762,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;
 	spinlock_t open_file_lock; /* protects list above */
 	struct cifs_ses *ses;	/* pointer to session associated with */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index c6bfe5b368f9..6421d8b433b1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -199,6 +199,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 54f507bd2c09..e06790171e89 100644
--- 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
@@ -2060,8 +2063,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;
 
@@ -2078,6 +2081,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);
@@ -2142,6 +2158,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,
@@ -2294,7 +2314,7 @@ cifs_put_smb_ses(struct cifs_ses *ses)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	sesInfoFree(ses);
-	cifs_put_tcp_session(server);
+	cifs_put_tcp_session(server, 0);
 }
 
 #ifdef CONFIG_KEYS
@@ -2467,7 +2487,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;
 	}
@@ -2557,7 +2577,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;
@@ -3606,7 +3626,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);
 	}
 
@@ -3904,7 +3924,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 1a6dde4bce62..ef1532360709 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1560,6 +1560,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)
 {
@@ -1572,32 +1620,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 5793f3e39a31..d45f772a35c9 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -89,6 +89,7 @@ extern int smb2_open_file(const unsigned int xid,
 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
-- 
2.11.0


  parent reply	other threads:[~2017-01-13  7:43 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-13  7:41 [patch added to 3.12-stable] Btrfs: fix tree search logic when replaying directory entry deletes Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] USB: serial: option: add support for Telit LE922A PIDs 0x1040, 0x1041 Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] USB: serial: option: add dlink dwm-158 Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] USB: serial: kl5kusb105: fix open error path Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] USB: cdc-acm: add device id for GW Instek AFG-125 Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] usb: hub: Fix auto-remount of safely removed or ejected USB-3 devices Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] usb: gadget: composite: correctly initialize ep->maxpacket Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] USB: UHCI: report non-PME wakeup signalling for Intel hardware Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ALSA: usb-audio: Add QuickCam Communicate Deluxe/S7500 to volume_control_quirks Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ALSA: hiface: Fix M2Tech hiFace driver sampling rate change Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ALSA: hda - Gate the mic jack on HP Z1 Gen3 AiO Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] Btrfs: fix memory leak in reading btree blocks Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] block_dev: don't test bdev->bd_contains when it is not stable Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] crypto: caam - fix AEAD givenc descriptors Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: fix mballoc breakage with 64k block size Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: fix stack memory corruption " Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: use more strict checks for inodes_per_block on mount Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: fix in-superblock mount options processing Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: add sanity checking to count_overhead() Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: reject inodes with negative size Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] ext4: return -ENOMEM instead of success Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] f2fs: set ->owner for debugfs status file's file_operations Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] mm/vmscan.c: set correct defer count for shrinker Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] fs: exec: apply CLOEXEC before changing dumpable task flags Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] usb: gadget: composite: always set ep->mult to a sensible value Jiri Slaby
2017-01-13  9:20   ` Felipe Balbi
2017-01-13  9:35     ` Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] dm crypt: mark key as invalid until properly loaded Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] dm space map metadata: fix 'struct sm_metadata' leak on failed create Jiri Slaby
2017-01-13  7:41 ` Jiri Slaby [this message]
2017-01-13  7:41 ` [patch added to 3.12-stable] CIFS: Fix missing nls unload in smb2_reconnect() Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] CIFS: Fix a possible memory corruption in push locks Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] xen/gntdev: Use VM_MIXEDMAP instead of VM_IO to avoid NUMA balancing Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] arm/xen: Use alloc_percpu rather than __alloc_percpu Jiri Slaby
2017-01-13  7:41 ` [patch added to 3.12-stable] xfs: set AGI buffer type in xlog_recover_clear_agi_bucket Jiri Slaby

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=20170113074159.26811-28-jslaby@suse.cz \
    --to=jslaby@suse.cz \
    --cc=pshilov@microsoft.com \
    --cc=stable@vger.kernel.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.