All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paulo Alcantara (SUSE)" <pc@cjr.nz>
To: smfrench@gmail.com
Cc: linux-cifs@vger.kernel.org, "Paulo Alcantara (SUSE)" <pc@cjr.nz>,
	Aurelien Aptel <aaptel@suse.com>
Subject: [PATCH v3 03/11] cifs: Fix potential softlockups while refreshing DFS cache
Date: Tue, 26 Nov 2019 21:36:26 -0300	[thread overview]
Message-ID: <20191127003634.14072-4-pc@cjr.nz> (raw)
In-Reply-To: <20191127003634.14072-1-pc@cjr.nz>

We used to skip reconnects on all SMB2_IOCTL commands due to SMB3+
FSCTL_VALIDATE_NEGOTIATE_INFO - which made sense since we're still
establishing a SMB session.

However, when refresh_cache_worker() calls smb2_get_dfs_refer() and
we're under reconnect, SMB2_ioctl() will not be able to get a proper
status error (e.g. -EHOSTDOWN in case we failed to reconnect) but an
-EAGAIN from cifs_send_recv() thus looping forever in
refresh_cache_worker().

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Suggested-by: Aurelien Aptel <aaptel@suse.com>
Fixes: e99c63e4d86d ("SMB3: Fix deadlock in validate negotiate hits reconnect")
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
---
 fs/cifs/smb2pdu.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index c09ca6963394..3cd90088d8ac 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -252,7 +252,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	if (tcon == NULL)
 		return 0;
 
-	if (smb2_command == SMB2_TREE_CONNECT || smb2_command == SMB2_IOCTL)
+	if (smb2_command == SMB2_TREE_CONNECT)
 		return 0;
 
 	if (tcon->tidStatus == CifsExiting) {
@@ -426,16 +426,9 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon, void *buf,
  * SMB information in the SMB header. If the return code is zero, this
  * function must have filled in request_buf pointer.
  */
-static int
-smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
-		    void **request_buf, unsigned int *total_len)
+static int __smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
+				  void **request_buf, unsigned int *total_len)
 {
-	int rc;
-
-	rc = smb2_reconnect(smb2_command, tcon);
-	if (rc)
-		return rc;
-
 	/* BB eventually switch this to SMB2 specific small buf size */
 	if (smb2_command == SMB2_SET_INFO)
 		*request_buf = cifs_buf_get();
@@ -456,7 +449,31 @@ smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
 		cifs_stats_inc(&tcon->num_smbs_sent);
 	}
 
-	return rc;
+	return 0;
+}
+
+static int smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon,
+			       void **request_buf, unsigned int *total_len)
+{
+	int rc;
+
+	rc = smb2_reconnect(smb2_command, tcon);
+	if (rc)
+		return rc;
+
+	return __smb2_plain_req_init(smb2_command, tcon, request_buf,
+				     total_len);
+}
+
+static int smb2_ioctl_req_init(u32 opcode, struct cifs_tcon *tcon,
+			       void **request_buf, unsigned int *total_len)
+{
+	/* Skip reconnect only for FSCTL_VALIDATE_NEGOTIATE_INFO IOCTLs */
+	if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) {
+		return __smb2_plain_req_init(SMB2_IOCTL, tcon, request_buf,
+					     total_len);
+	}
+	return smb2_plain_req_init(SMB2_IOCTL, tcon, request_buf, total_len);
 }
 
 /* For explanation of negotiate contexts see MS-SMB2 section 2.2.3.1 */
@@ -2686,7 +2703,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst,
 	int rc;
 	char *in_data_buf;
 
-	rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len);
+	rc = smb2_ioctl_req_init(opcode, tcon, (void **) &req, &total_len);
 	if (rc)
 		return rc;
 
-- 
2.24.0


  parent reply	other threads:[~2019-11-27  0:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-27  0:36 [PATCH v3 00/11] DFS fixes Paulo Alcantara (SUSE)
2019-11-27  0:36 ` [PATCH v3 01/11] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE)
2019-11-27  0:36 ` [PATCH v3 02/11] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE)
2019-11-27  0:36 ` Paulo Alcantara (SUSE) [this message]
2019-11-27  0:36 ` [PATCH v3 04/11] cifs: Clean up " Paulo Alcantara (SUSE)
2019-11-27 11:32   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 05/11] cifs: Get rid of kstrdup_const()'d paths Paulo Alcantara (SUSE)
2019-11-27 11:33   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 06/11] cifs: Introduce helpers for finding TCP connection Paulo Alcantara (SUSE)
2019-11-27 11:33   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 07/11] cifs: Merge is_path_valid() into get_normalized_path() Paulo Alcantara (SUSE)
2019-11-27 11:34   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 08/11] cifs: Fix potential deadlock when updating vol in cifs_reconnect() Paulo Alcantara (SUSE)
2019-11-27 12:09   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 09/11] cifs: Avoid doing network I/O while holding cache lock Paulo Alcantara (SUSE)
2019-11-27 13:32   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 10/11] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE)
2019-11-27 13:33   ` Aurélien Aptel
2019-11-27  0:36 ` [PATCH v3 11/11] cifs: Always update signing key of first channel Paulo Alcantara (SUSE)

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=20191127003634.14072-4-pc@cjr.nz \
    --to=pc@cjr.nz \
    --cc=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=smfrench@gmail.com \
    /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.