[PATCH v2 0/6] Add support for DFS in SMB2+

[PATCH v2 0/6] Add support for DFS in SMB2+

[Aurelien Aptel]

This series of patch tries to implement the get_dfs_refer operation
for SMB2+.

In SMB2+, DFS resolving is now done through an FSCTL (patch #3). The
relevant Microsoft specifications for this are:

MS-SMB2: 3.2.4.20   Application Requests an IO Control Code Operation
MS-SMB2: 3.2.4.20.3 Application Requests DFS Referral Information
MS-SMB2: 3.2.5.14   Receiving an SMB2 IOCTL Response
MS-SMB2: 3.2.5.14.4 Handling a DFS Referral Information Response

MS-DFSC: 2.2  Message Syntax (but really the whole document is useful)

The DFS response payload however is identical. Patch #1 moves the
DFS response parsing out of SMB1 code and makes it work on any version
of SMB.

DFS code has 2 "main" entry points: initial mounting and automount
(when a DFS link is accessed/traversed).

When automounting, cifs.ko calls build_path_from_dentry() which only
makes treename-prefixed paths when the tcon has the
SMB_IN_DFS_SHARE. This flag is checked in tcon->Flags which is never
touched by SMB2 code as it sets tcon->share_flags on connexion.

* CIFS requires to prefix all pathnames with the treename prefix when
  connected to a DFS server, so the current build_path_from_dentry()
  makes sense for CIFS. 
* For SMB2+ it seems only the Create request requires the treename
  prefix. Simply making the function check for both CIFS SMB2+ flag
  for DFS to add a prefix does not work as the server has different
  expectations about which packet can have/require a DFS pathname.

Patch #2 adds build_path_from_dentry_optional_prefix() with an extra
bool arg to decide to prefix or not. The automount code path always
ask for it. Patch #5 modifies SMB2_open() to add the treename prefix
to the given path if the server requires it.

As part of the mouting process a IPC tcon is made (I suspect we don't
need it anymore in SMB3). This tcon doesn't respect the signing
requirement. This is fixed by patch #4.

Finally the SMB2+ implementation of the get_dfs_referral operation is
added in all supported SMB versions in patch #6.

I've sucessfuly tested this against a Windows Server 2016 DFS
setup. Samba requires a patch as currently it only accepts DFS
referrals requests on an IPC connexion, which is in violation of the
spec. A patch was sent on samba-technical:

https://lists.samba.org/archive/samba-technical/2017-February/118859.html

Changes since v1:

* The fsctl is made on the first available tcon to the server, which
  is allowed by the spec. This way we avoid hardcoding additional
  rules in SMB2_ioctl().
* add signing flag in smb2 header if required
* no copy/pasting, the DFS parsing code is now generic and works for
  both SMB versions
* add treename prefix in SMB2_open() when required

Aurelien Aptel (6):
  CIFS: move DFS response parsing out of SMB1 code
  CIFS: add build_path_from_dentry_optional_prefix()
  CIFS: implement get_dfs_refer for SMB2+
  CIFS: set signing flag in SMB2+ TreeConnect if needed
  CIFS: use DFS pathnames in SMB2+ Create requests
  CIFS: enable get_dfs_refer for SMB2+

 fs/cifs/cifs_dfs_ref.c |   4 +-
 fs/cifs/cifspdu.h      |  16 ++++---
 fs/cifs/cifsproto.h    |   7 +++
 fs/cifs/cifssmb.c      | 119 +++----------------------------------------------
 fs/cifs/dir.c          |  13 +++++-
 fs/cifs/misc.c         | 105 +++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2ops.c      |  76 +++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c      |  97 ++++++++++++++++++++++++++++++++--------
 fs/cifs/smb2pdu.h      |   8 ++++
 9 files changed, 305 insertions(+), 140 deletions(-)

-- 
2.10.2

[PATCH v2 1/6] CIFS: move DFS response parsing out of SMB1 code

[Aurelien Aptel]

since the DFS payload is not tied to the SMB version we can:
* isolate the DFS payload in its own struct, and include that struct in
  packet structs
* move the function that parses the response to misc.c and make it work
  on the new DFS payload struct (add payload size and utf16 flag as a
  result).

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifspdu.h   |  16 ++++---
 fs/cifs/cifsproto.h |   5 +++
 fs/cifs/cifssmb.c   | 119 +++-------------------------------------------------
 fs/cifs/misc.c      | 105 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+), 120 deletions(-)

diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index f5b8730..1ce733f 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -2086,17 +2086,21 @@ typedef struct dfs_referral_level_3 { /* version 4 is same, + one flag bit */
 	__u8   ServiceSiteGuid[16];  /* MBZ, ignored */
 } __attribute__((packed)) REFERRAL3;
 
-typedef struct smb_com_transaction_get_dfs_refer_rsp {
-	struct smb_hdr hdr;	/* wct = 10 */
-	struct trans2_resp t2;
-	__u16 ByteCount;
-	__u8 Pad;
+struct get_dfs_referral_rsp {
 	__le16 PathConsumed;
 	__le16 NumberOfReferrals;
 	__le32 DFSFlags;
 	REFERRAL3 referrals[1];	/* array of level 3 dfs_referral structures */
 	/* followed by the strings pointed to by the referral structures */
-} __attribute__((packed)) TRANSACTION2_GET_DFS_REFER_RSP;
+} __packed;
+
+typedef struct smb_com_transaction_get_dfs_refer_rsp {
+	struct smb_hdr hdr;	/* wct = 10 */
+	struct trans2_resp t2;
+	__u16 ByteCount;
+	__u8 Pad;
+	struct get_dfs_referral_rsp dfs_data;
+} __packed TRANSACTION2_GET_DFS_REFER_RSP;
 
 /* DFS Flags */
 #define DFSREF_REFERRAL_SERVER  0x00000001 /* all targets are DFS roots */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 406d2c1..c097830 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -284,6 +284,11 @@ extern int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
 			const struct nls_table *nls_codepage,
 			unsigned int *num_referrals,
 			struct dfs_info3_param **referrals, int remap);
+extern int parse_dfs_referrals(struct get_dfs_referral_rsp *rsp, u32 rsp_size,
+			       unsigned int *num_of_nodes,
+			       struct dfs_info3_param **target_nodes,
+			       const struct nls_table *nls_codepage, int remap,
+			       const char *searchName, bool is_unicode);
 extern void reset_cifs_unix_caps(unsigned int xid, struct cifs_tcon *tcon,
 				 struct cifs_sb_info *cifs_sb,
 				 struct smb_vol *vol);
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index f5099fb..5005c79 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4786,117 +4786,6 @@ CIFSGetSrvInodeNumber(const unsigned int xid, struct cifs_tcon *tcon,
 	return rc;
 }
 
-/* parses DFS refferal V3 structure
- * caller is responsible for freeing target_nodes
- * returns:
- * 	on success - 0
- *	on failure - errno
- */
-static int
-parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
-		unsigned int *num_of_nodes,
-		struct dfs_info3_param **target_nodes,
-		const struct nls_table *nls_codepage, int remap,
-		const char *searchName)
-{
-	int i, rc = 0;
-	char *data_end;
-	bool is_unicode;
-	struct dfs_referral_level_3 *ref;
-
-	if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE)
-		is_unicode = true;
-	else
-		is_unicode = false;
-	*num_of_nodes = le16_to_cpu(pSMBr->NumberOfReferrals);
-
-	if (*num_of_nodes < 1) {
-		cifs_dbg(VFS, "num_referrals: must be at least > 0, but we get num_referrals = %d\n",
-			 *num_of_nodes);
-		rc = -EINVAL;
-		goto parse_DFS_referrals_exit;
-	}
-
-	ref = (struct dfs_referral_level_3 *) &(pSMBr->referrals);
-	if (ref->VersionNumber != cpu_to_le16(3)) {
-		cifs_dbg(VFS, "Referrals of V%d version are not supported, should be V3\n",
-			 le16_to_cpu(ref->VersionNumber));
-		rc = -EINVAL;
-		goto parse_DFS_referrals_exit;
-	}
-
-	/* get the upper boundary of the resp buffer */
-	data_end = (char *)(&(pSMBr->PathConsumed)) +
-				le16_to_cpu(pSMBr->t2.DataCount);
-
-	cifs_dbg(FYI, "num_referrals: %d dfs flags: 0x%x ...\n",
-		 *num_of_nodes, le32_to_cpu(pSMBr->DFSFlags));
-
-	*target_nodes = kcalloc(*num_of_nodes, sizeof(struct dfs_info3_param),
-				GFP_KERNEL);
-	if (*target_nodes == NULL) {
-		rc = -ENOMEM;
-		goto parse_DFS_referrals_exit;
-	}
-
-	/* collect necessary data from referrals */
-	for (i = 0; i < *num_of_nodes; i++) {
-		char *temp;
-		int max_len;
-		struct dfs_info3_param *node = (*target_nodes)+i;
-
-		node->flags = le32_to_cpu(pSMBr->DFSFlags);
-		if (is_unicode) {
-			__le16 *tmp = kmalloc(strlen(searchName)*2 + 2,
-						GFP_KERNEL);
-			if (tmp == NULL) {
-				rc = -ENOMEM;
-				goto parse_DFS_referrals_exit;
-			}
-			cifsConvertToUTF16((__le16 *) tmp, searchName,
-					   PATH_MAX, nls_codepage, remap);
-			node->path_consumed = cifs_utf16_bytes(tmp,
-					le16_to_cpu(pSMBr->PathConsumed),
-					nls_codepage);
-			kfree(tmp);
-		} else
-			node->path_consumed = le16_to_cpu(pSMBr->PathConsumed);
-
-		node->server_type = le16_to_cpu(ref->ServerType);
-		node->ref_flag = le16_to_cpu(ref->ReferralEntryFlags);
-
-		/* copy DfsPath */
-		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
-		max_len = data_end - temp;
-		node->path_name = cifs_strndup_from_utf16(temp, max_len,
-						is_unicode, nls_codepage);
-		if (!node->path_name) {
-			rc = -ENOMEM;
-			goto parse_DFS_referrals_exit;
-		}
-
-		/* copy link target UNC */
-		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
-		max_len = data_end - temp;
-		node->node_name = cifs_strndup_from_utf16(temp, max_len,
-						is_unicode, nls_codepage);
-		if (!node->node_name) {
-			rc = -ENOMEM;
-			goto parse_DFS_referrals_exit;
-		}
-
-		ref++;
-	}
-
-parse_DFS_referrals_exit:
-	if (rc) {
-		free_dfs_info_array(*target_nodes, *num_of_nodes);
-		*target_nodes = NULL;
-		*num_of_nodes = 0;
-	}
-	return rc;
-}
-
 int
 CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 		const char *search_name, struct dfs_info3_param **target_nodes,
@@ -4993,9 +4882,11 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 		 get_bcc(&pSMBr->hdr), le16_to_cpu(pSMBr->t2.DataOffset));
 
 	/* parse returned result into more usable form */
-	rc = parse_DFS_referrals(pSMBr, num_of_nodes,
-				 target_nodes, nls_codepage, remap,
-				 search_name);
+	rc = parse_dfs_referrals(&pSMBr->dfs_data,
+				 le16_to_cpu(pSMBr->t2.DataCount),
+				 num_of_nodes, target_nodes, nls_codepage,
+				 remap, search_name,
+				 pSMBr->hdr.Flags2 & SMBFLG2_UNICODE);
 
 GetDFSRefExit:
 	cifs_buf_release(pSMB);
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index c672915..d3fb115 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -640,3 +640,108 @@ cifs_add_pending_open(struct cifs_fid *fid, struct tcon_link *tlink,
 	cifs_add_pending_open_locked(fid, tlink, open);
 	spin_unlock(&tlink_tcon(open->tlink)->open_file_lock);
 }
+
+/* parses DFS refferal V3 structure
+ * caller is responsible for freeing target_nodes
+ * returns:
+ * - on success - 0
+ * - on failure - errno
+ */
+int
+parse_dfs_referrals(struct get_dfs_referral_rsp *rsp, u32 rsp_size,
+		    unsigned int *num_of_nodes,
+		    struct dfs_info3_param **target_nodes,
+		    const struct nls_table *nls_codepage, int remap,
+		    const char *searchName, bool is_unicode)
+{
+	int i, rc = 0;
+	char *data_end;
+	struct dfs_referral_level_3 *ref;
+
+	*num_of_nodes = le16_to_cpu(rsp->NumberOfReferrals);
+
+	if (*num_of_nodes < 1) {
+		cifs_dbg(VFS, "num_referrals: must be at least > 0, but we get num_referrals = %d\n",
+			 *num_of_nodes);
+		rc = -EINVAL;
+		goto parse_DFS_referrals_exit;
+	}
+
+	ref = (struct dfs_referral_level_3 *) &(rsp->referrals);
+	if (ref->VersionNumber != cpu_to_le16(3)) {
+		cifs_dbg(VFS, "Referrals of V%d version are not supported, should be V3\n",
+			 le16_to_cpu(ref->VersionNumber));
+		rc = -EINVAL;
+		goto parse_DFS_referrals_exit;
+	}
+
+	/* get the upper boundary of the resp buffer */
+	data_end = (char *)rsp + rsp_size;
+
+	cifs_dbg(FYI, "num_referrals: %d dfs flags: 0x%x ...\n",
+		 *num_of_nodes, le32_to_cpu(rsp->DFSFlags));
+
+	*target_nodes = kcalloc(*num_of_nodes, sizeof(struct dfs_info3_param),
+				GFP_KERNEL);
+	if (*target_nodes == NULL) {
+		rc = -ENOMEM;
+		goto parse_DFS_referrals_exit;
+	}
+
+	/* collect necessary data from referrals */
+	for (i = 0; i < *num_of_nodes; i++) {
+		char *temp;
+		int max_len;
+		struct dfs_info3_param *node = (*target_nodes)+i;
+
+		node->flags = le32_to_cpu(rsp->DFSFlags);
+		if (is_unicode) {
+			__le16 *tmp = kmalloc(strlen(searchName)*2 + 2,
+						GFP_KERNEL);
+			if (tmp == NULL) {
+				rc = -ENOMEM;
+				goto parse_DFS_referrals_exit;
+			}
+			cifsConvertToUTF16((__le16 *) tmp, searchName,
+					   PATH_MAX, nls_codepage, remap);
+			node->path_consumed = cifs_utf16_bytes(tmp,
+					le16_to_cpu(rsp->PathConsumed),
+					nls_codepage);
+			kfree(tmp);
+		} else
+			node->path_consumed = le16_to_cpu(rsp->PathConsumed);
+
+		node->server_type = le16_to_cpu(ref->ServerType);
+		node->ref_flag = le16_to_cpu(ref->ReferralEntryFlags);
+
+		/* copy DfsPath */
+		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
+		max_len = data_end - temp;
+		node->path_name = cifs_strndup_from_utf16(temp, max_len,
+						is_unicode, nls_codepage);
+		if (!node->path_name) {
+			rc = -ENOMEM;
+			goto parse_DFS_referrals_exit;
+		}
+
+		/* copy link target UNC */
+		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
+		max_len = data_end - temp;
+		node->node_name = cifs_strndup_from_utf16(temp, max_len,
+						is_unicode, nls_codepage);
+		if (!node->node_name) {
+			rc = -ENOMEM;
+			goto parse_DFS_referrals_exit;
+		}
+
+		ref++;
+	}
+
+parse_DFS_referrals_exit:
+	if (rc) {
+		free_dfs_info_array(*target_nodes, *num_of_nodes);
+		*target_nodes = NULL;
+		*num_of_nodes = 0;
+	}
+	return rc;
+}
-- 
2.10.2

[PATCH v2 2/6] CIFS: add build_path_from_dentry_optional_prefix()

[Aurelien Aptel]

this function does the same thing as add build_path_from_dentry() but
takes a boolean parameter to decide whether or not to prefix the path
with the tree name.

we cannot rely on tcon->Flags & SMB_SHARE_IS_IN_DFS for SMB2 as smb2
code never sets tcon->Flags but it sets tcon->share_flags and it seems
the SMB_SHARE_IS_IN_DFS has different semantics in SMB2: the prefix
shouldn't be added everytime it was in SMB1.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifs_dfs_ref.c |  4 +++-
 fs/cifs/cifsproto.h    |  2 ++
 fs/cifs/dir.c          | 13 ++++++++++++-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index ec9dbbc..8a9ebfd 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -302,7 +302,9 @@ static struct vfsmount *cifs_dfs_do_automount(struct dentry *mntpt)
 	 * gives us the latter, so we must adjust the result.
 	 */
 	mnt = ERR_PTR(-ENOMEM);
-	full_path = build_path_from_dentry(mntpt);
+
+	/* always use tree name prefix */
+	full_path = build_path_from_dentry_optional_prefix(mntpt, true);
 	if (full_path == NULL)
 		goto cdda_exit;
 
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index c097830..9ee46c1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -61,6 +61,8 @@ extern void exit_cifs_idmap(void);
 extern int init_cifs_spnego(void);
 extern void exit_cifs_spnego(void);
 extern char *build_path_from_dentry(struct dentry *);
+extern char *build_path_from_dentry_optional_prefix(struct dentry *direntry,
+						    bool prefix);
 extern char *cifs_build_path_to_root(struct smb_vol *vol,
 				     struct cifs_sb_info *cifs_sb,
 				     struct cifs_tcon *tcon,
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 2c227a9..56366e9 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -81,6 +81,17 @@ cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
 char *
 build_path_from_dentry(struct dentry *direntry)
 {
+	struct cifs_sb_info *cifs_sb = CIFS_SB(direntry->d_sb);
+	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
+	bool prefix = tcon->Flags & SMB_SHARE_IS_IN_DFS;
+
+	return build_path_from_dentry_optional_prefix(direntry,
+						      prefix);
+}
+
+char *
+build_path_from_dentry_optional_prefix(struct dentry *direntry, bool prefix)
+{
 	struct dentry *temp;
 	int namelen;
 	int dfsplen;
@@ -92,7 +103,7 @@ build_path_from_dentry(struct dentry *direntry)
 	unsigned seq;
 
 	dirsep = CIFS_DIR_SEP(cifs_sb);
-	if (tcon->Flags & SMB_SHARE_IS_IN_DFS)
+	if (prefix)
 		dfsplen = strnlen(tcon->treeName, MAX_TREE_SIZE + 1);
 	else
 		dfsplen = 0;
-- 
2.10.2

[PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Aurelien Aptel]

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2ops.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.h |  8 +++++++
 2 files changed, 80 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index a44b4db..2563fe8 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1097,6 +1097,78 @@ smb2_new_lease_key(struct cifs_fid *fid)
 	generate_random_uuid(fid->lease_key);
 }
 
+static int
+smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
+		   const char *search_name,
+		   struct dfs_info3_param **target_nodes,
+		   unsigned int *num_of_nodes,
+		   const struct nls_table *nls_codepage, int remap)
+{
+	int rc = -EINVAL;
+	__le16 *utf16_path = NULL;
+	int utf16_path_len = 0;
+	struct cifs_tcon *tcon;
+	struct fsctl_get_dfs_referral_req *dfs_req = NULL;
+	struct get_dfs_referral_rsp *dfs_rsp = NULL;
+	u32 dfs_req_size = 0, dfs_rsp_size = 0;
+
+	cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
+
+	/*
+	 * Use any tcon from the current session. Here, the first one.
+	 */
+	tcon = list_first_entry(&ses->tcon_list, struct cifs_tcon, tcon_list);
+	if (!tcon)
+		goto out;
+
+	utf16_path = cifs_strndup_to_utf16(search_name, PATH_MAX,
+					   &utf16_path_len,
+					   nls_codepage, remap);
+	if (!utf16_path) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	dfs_req_size = sizeof(*dfs_req) + utf16_path_len;
+	dfs_req = kzalloc(dfs_req_size, GFP_KERNEL);
+	if (!dfs_req) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	/* Highest DFS referral version understood */
+	dfs_req->MaxReferralLevel = cpu_to_le16(DFS_VERSION);
+
+	/* Path to resolve in an UTF-16 null-terminated string */
+	memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
+
+	do {
+		rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
+				FSCTL_DFS_GET_REFERRALS, true /* is_fsctl */,
+				(char *)dfs_req, dfs_req_size,
+				(char **)&dfs_rsp, &dfs_rsp_size);
+	} while (rc == -EAGAIN);
+
+	if (rc) {
+		cifs_dbg(VFS, "ioctl error in smb2_get_dfs_refer rc=%d\n", rc);
+		goto out;
+	}
+
+	rc = parse_dfs_referrals(dfs_rsp, dfs_rsp_size,
+				 num_of_nodes, target_nodes,
+				 nls_codepage, remap, search_name,
+				 true /* is_unicode */);
+	if (rc) {
+		cifs_dbg(VFS, "parse error in smb2_get_dfs_refer rc=%d\n", rc);
+		goto out;
+	}
+
+ out:
+	kfree(utf16_path);
+	kfree(dfs_req);
+	kfree(dfs_rsp);
+	return rc;
+}
 #define SMB2_SYMLINK_STRUCT_SIZE \
 	(sizeof(struct smb2_err_rsp) - 1 + sizeof(struct smb2_symlink_err_rsp))
 
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index c03b252..18700fd 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -695,6 +695,14 @@ struct fsctl_get_integrity_information_rsp {
 /* Integrity flags for above */
 #define FSCTL_INTEGRITY_FLAG_CHECKSUM_ENFORCEMENT_OFF	0x00000001
 
+/* See MS-DFSC 2.2.2 */
+struct fsctl_get_dfs_referral_req {
+	__le16 MaxReferralLevel;
+	__u8 RequestFileName[];
+} __packed;
+
+/* DFS response is struct get_dfs_refer_rsp */
+
 /* See MS-SMB2 2.2.31.3 */
 struct network_resiliency_req {
 	__le32 Timeout;
-- 
2.10.2

[PATCH v2 4/6] CIFS: set signing flag in SMB2+ TreeConnect if needed

[Aurelien Aptel]

cifs_enable_signing() already sets server->sign according to what the
server requires/offers and what mount options allows/forbids, so use
that.

this is required for IPC tcon that connects to signing-required servers.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index ad83b3d..c9a1d71 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1167,8 +1167,8 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 
 		/* since no tcon, smb2_init can not do this, so do here */
 		req->hdr.sync_hdr.SessionId = ses->Suid;
-		/* if (ses->server->sec_mode & SECMODE_SIGN_REQUIRED)
-			req->hdr.Flags |= SMB2_FLAGS_SIGNED; */
+		if (ses->server->sign)
+			req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
 	} else if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
-- 
2.10.2

[PATCH v2 5/6] CIFS: use DFS pathnames in SMB2+ Create requests

[Aurelien Aptel]

When connected to a DFS capable share, the client must set the
SMB2_FLAGS_DFS_OPERATIONS flag in the SMB2 header and use
DFS path names: "<server>\<share>\<path>" *without* leading \\.

Sources:

[MS-SMB2] 3.2.5.5 Receiving an SMB2 TREE_CONNECT Response
> TreeConnect.IsDfsShare MUST be set to TRUE, if the SMB2_SHARE_CAP_DFS
> bit is set in the Capabilities field of the response.

[MS-SMB2] 3.2.4.3 Application Requests Opening a File
> If TreeConnect.IsDfsShare is TRUE, the SMB2_FLAGS_DFS_OPERATIONS flag
> is set in the Flags field.

[MS-SMB2] 2.2.13 SMB2 CREATE Request, NameOffset:
> If SMB2_FLAGS_DFS_OPERATIONS is set in the Flags field of the SMB2
> header, the file name includes a prefix that will be processed during
> DFS name normalization as specified in section 3.3.5.9. Otherwise, the
> file name is relative to the share that is identified by the TreeId in
> the SMB2 header.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2pdu.c | 93 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 77 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index c9a1d71..9e9f449 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1527,6 +1527,48 @@ add_durable_context(struct kvec *iov, unsigned int *num_iovec,
 	return 0;
 }
 
+static int
+alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len,
+			    const char *treename, const __le16 *path)
+{
+	int treename_len, path_len;
+	const __le16 sep[] = {cpu_to_le16('\\'), cpu_to_le16(0x0000)};
+
+	/*
+	 * skip leading "\\"
+	 */
+	treename_len = strlen(treename);
+	if (treename_len < 2 || !(treename[0] == '\\' && treename[1] == '\\'))
+		return -EINVAL;
+
+	treename += 2;
+	treename_len -= 2;
+
+	path_len = UniStrnlen((wchar_t *)path, PATH_MAX);
+
+	/*
+	 * make room for one path separator between the treename and
+	 * path
+	 */
+	*out_len = treename_len + 1 + path_len;
+
+	/*
+	 * final path needs to be null-terminated UTF16 with a
+	 * size aligned to 8
+	 */
+
+	*out_size = roundup((*out_len+1)*2, 8);
+	*out_path = kzalloc(*out_size, GFP_KERNEL);
+	if (!*out_path)
+		return -ENOMEM;
+
+	cifs_strtoUTF16(*out_path, treename, treename_len, load_nls_default());
+	UniStrcat(*out_path, sep);
+	UniStrcat(*out_path, path);
+
+	return 0;
+}
+
 int
 SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 	  __u8 *oplock, struct smb2_file_all_info *buf,
@@ -1575,30 +1617,49 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
 	req->ShareAccess = FILE_SHARE_ALL_LE;
 	req->CreateDisposition = cpu_to_le32(oparms->disposition);
 	req->CreateOptions = cpu_to_le32(oparms->create_options & CREATE_OPTIONS_MASK);
-	uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
-	/* do not count rfc1001 len field */
-	req->NameOffset = cpu_to_le16(sizeof(struct smb2_create_req) - 4);
 
 	iov[0].iov_base = (char *)req;
 	/* 4 for rfc1002 length field */
 	iov[0].iov_len = get_rfc1002_length(req) + 4;
-
-	/* MUST set path len (NameLength) to 0 opening root of share */
-	req->NameLength = cpu_to_le16(uni_path_len - 2);
 	/* -1 since last byte is buf[0] which is sent below (path) */
 	iov[0].iov_len--;
-	if (uni_path_len % 8 != 0) {
-		copy_size = uni_path_len / 8 * 8;
-		if (copy_size < uni_path_len)
-			copy_size += 8;
-
-		copy_path = kzalloc(copy_size, GFP_KERNEL);
-		if (!copy_path)
-			return -ENOMEM;
-		memcpy((char *)copy_path, (const char *)path,
-			uni_path_len);
+
+	req->NameOffset = cpu_to_le16(sizeof(struct smb2_create_req) - 4);
+
+	/* [MS-SMB2] 2.2.13 NameOffset:
+	 * If SMB2_FLAGS_DFS_OPERATIONS is set in the Flags field of
+	 * the SMB2 header, the file name includes a prefix that will
+	 * be processed during DFS name normalization as specified in
+	 * section 3.3.5.9. Otherwise, the file name is relative to
+	 * the share that is identified by the TreeId in the SMB2
+	 * header.
+	 */
+	if (tcon->share_flags & SHI1005_FLAGS_DFS) {
+		int name_len;
+
+		req->hdr.sync_hdr.Flags |= SMB2_FLAGS_DFS_OPERATIONS;
+		rc = alloc_path_with_tree_prefix(&copy_path, &copy_size,
+						 &name_len,
+						 tcon->treeName, path);
+		if (rc)
+			return rc;
+		req->NameLength = cpu_to_le16(name_len * 2);
 		uni_path_len = copy_size;
 		path = copy_path;
+	} else {
+		uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
+		/* MUST set path len (NameLength) to 0 opening root of share */
+		req->NameLength = cpu_to_le16(uni_path_len - 2);
+		if (uni_path_len % 8 != 0) {
+			copy_size = roundup(uni_path_len, 8);
+			copy_path = kzalloc(copy_size, GFP_KERNEL);
+			if (!copy_path)
+				return -ENOMEM;
+			memcpy((char *)copy_path, (const char *)path,
+			       uni_path_len);
+			uni_path_len = copy_size;
+			path = copy_path;
+		}
 	}
 
 	iov[1].iov_len = uni_path_len;
-- 
2.10.2

[PATCH v2 6/6] CIFS: enable get_dfs_refer for SMB2+

[Aurelien Aptel]

Add the newly defined operation to the relevant structures.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2ops.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 2563fe8..1537cc8 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2326,6 +2326,7 @@ struct smb_version_operations smb20_operations = {
 	.clone_range = smb2_clone_range,
 	.wp_retry_size = smb2_wp_retry_size,
 	.dir_needs_close = smb2_dir_needs_close,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 struct smb_version_operations smb21_operations = {
@@ -2407,6 +2408,7 @@ struct smb_version_operations smb21_operations = {
 	.wp_retry_size = smb2_wp_retry_size,
 	.dir_needs_close = smb2_dir_needs_close,
 	.enum_snapshots = smb3_enum_snapshots,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 struct smb_version_operations smb30_operations = {
@@ -2498,6 +2500,7 @@ struct smb_version_operations smb30_operations = {
 	.free_transform_rq = smb3_free_transform_rq,
 	.is_transform_hdr = smb3_is_transform_hdr,
 	.receive_transform = smb3_receive_transform,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 
 #ifdef CONFIG_CIFS_SMB311
@@ -2590,6 +2593,7 @@ struct smb_version_operations smb311_operations = {
 	.free_transform_rq = smb3_free_transform_rq,
 	.is_transform_hdr = smb3_is_transform_hdr,
 	.receive_transform = smb3_receive_transform,
+	.get_dfs_refer = smb2_get_dfs_refer,
 };
 #endif /* CIFS_SMB311 */
 
-- 
2.10.2

Re: [PATCH v2 0/6] Add support for DFS in SMB2+

[Pavel Shilovsky]

2017-02-23 6:43 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> This series of patch tries to implement the get_dfs_refer operation
> for SMB2+.
>
> In SMB2+, DFS resolving is now done through an FSCTL (patch #3). The
> relevant Microsoft specifications for this are:
>
> MS-SMB2: 3.2.4.20   Application Requests an IO Control Code Operation
> MS-SMB2: 3.2.4.20.3 Application Requests DFS Referral Information
> MS-SMB2: 3.2.5.14   Receiving an SMB2 IOCTL Response
> MS-SMB2: 3.2.5.14.4 Handling a DFS Referral Information Response
>
> MS-DFSC: 2.2  Message Syntax (but really the whole document is useful)
>
> The DFS response payload however is identical. Patch #1 moves the
> DFS response parsing out of SMB1 code and makes it work on any version
> of SMB.
>
> DFS code has 2 "main" entry points: initial mounting and automount
> (when a DFS link is accessed/traversed).
>
> When automounting, cifs.ko calls build_path_from_dentry() which only
> makes treename-prefixed paths when the tcon has the
> SMB_IN_DFS_SHARE. This flag is checked in tcon->Flags which is never
> touched by SMB2 code as it sets tcon->share_flags on connexion.
>
> * CIFS requires to prefix all pathnames with the treename prefix when
>   connected to a DFS server, so the current build_path_from_dentry()
>   makes sense for CIFS.
> * For SMB2+ it seems only the Create request requires the treename
>   prefix. Simply making the function check for both CIFS SMB2+ flag
>   for DFS to add a prefix does not work as the server has different
>   expectations about which packet can have/require a DFS pathname.
>
> Patch #2 adds build_path_from_dentry_optional_prefix() with an extra
> bool arg to decide to prefix or not. The automount code path always
> ask for it. Patch #5 modifies SMB2_open() to add the treename prefix
> to the given path if the server requires it.
>
> As part of the mouting process a IPC tcon is made (I suspect we don't
> need it anymore in SMB3). This tcon doesn't respect the signing
> requirement. This is fixed by patch #4.
>
> Finally the SMB2+ implementation of the get_dfs_referral operation is
> added in all supported SMB versions in patch #6.
>
> I've sucessfuly tested this against a Windows Server 2016 DFS
> setup. Samba requires a patch as currently it only accepts DFS
> referrals requests on an IPC connexion, which is in violation of the
> spec. A patch was sent on samba-technical:
>
> https://lists.samba.org/archive/samba-technical/2017-February/118859.html
>
> Changes since v1:
>
> * The fsctl is made on the first available tcon to the server, which
>   is allowed by the spec. This way we avoid hardcoding additional
>   rules in SMB2_ioctl().
> * add signing flag in smb2 header if required
> * no copy/pasting, the DFS parsing code is now generic and works for
>   both SMB versions
> * add treename prefix in SMB2_open() when required
>
> Aurelien Aptel (6):
>   CIFS: move DFS response parsing out of SMB1 code
>   CIFS: add build_path_from_dentry_optional_prefix()
>   CIFS: implement get_dfs_refer for SMB2+
>   CIFS: set signing flag in SMB2+ TreeConnect if needed
>   CIFS: use DFS pathnames in SMB2+ Create requests
>   CIFS: enable get_dfs_refer for SMB2+
>
>  fs/cifs/cifs_dfs_ref.c |   4 +-
>  fs/cifs/cifspdu.h      |  16 ++++---
>  fs/cifs/cifsproto.h    |   7 +++
>  fs/cifs/cifssmb.c      | 119 +++----------------------------------------------
>  fs/cifs/dir.c          |  13 +++++-
>  fs/cifs/misc.c         | 105 +++++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/smb2ops.c      |  76 +++++++++++++++++++++++++++++++
>  fs/cifs/smb2pdu.c      |  97 ++++++++++++++++++++++++++++++++--------
>  fs/cifs/smb2pdu.h      |   8 ++++
>  9 files changed, 305 insertions(+), 140 deletions(-)
>
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

I would merge patches #3 and #6 because a static function which is
introduced in #3 should raise compiler warnings. Other than that the
patchset looks good.

Reviewed-by: Pavel Shilovsky <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>

-- 
Best regards,
Pavel Shilovsky

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Pavel Shilovsky]

2017-02-23 6:43 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/smb2ops.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/smb2pdu.h |  8 +++++++
>  2 files changed, 80 insertions(+)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index a44b4db..2563fe8 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1097,6 +1097,78 @@ smb2_new_lease_key(struct cifs_fid *fid)
>         generate_random_uuid(fid->lease_key);
>  }
>
> +static int
> +smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
> +                  const char *search_name,
> +                  struct dfs_info3_param **target_nodes,
> +                  unsigned int *num_of_nodes,
> +                  const struct nls_table *nls_codepage, int remap)
> +{
> +       int rc = -EINVAL;
> +       __le16 *utf16_path = NULL;
> +       int utf16_path_len = 0;
> +       struct cifs_tcon *tcon;
> +       struct fsctl_get_dfs_referral_req *dfs_req = NULL;
> +       struct get_dfs_referral_rsp *dfs_rsp = NULL;
> +       u32 dfs_req_size = 0, dfs_rsp_size = 0;
> +
> +       cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
> +
> +       /*
> +        * Use any tcon from the current session. Here, the first one.
> +        */
> +       tcon = list_first_entry(&ses->tcon_list, struct cifs_tcon, tcon_list);
> +       if (!tcon)
> +               goto out;

Accessing the tcon_list should be protected by spin_locks and also a
reference to the tcon should be acquired before using it safely.

Also you can extend SMB2_ioctl argument list to allow passing ipc_tid.
In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if
tcon is passed -use it, otherwise - use ipc_tid.

In this case you can fallback to ipc_tid if no tcon is found in the list.

> +
> +       utf16_path = cifs_strndup_to_utf16(search_name, PATH_MAX,
> +                                          &utf16_path_len,
> +                                          nls_codepage, remap);
> +       if (!utf16_path) {
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       dfs_req_size = sizeof(*dfs_req) + utf16_path_len;
> +       dfs_req = kzalloc(dfs_req_size, GFP_KERNEL);
> +       if (!dfs_req) {
> +               rc = -ENOMEM;
> +               goto out;
> +       }
> +
> +       /* Highest DFS referral version understood */
> +       dfs_req->MaxReferralLevel = cpu_to_le16(DFS_VERSION);
> +
> +       /* Path to resolve in an UTF-16 null-terminated string */
> +       memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
> +
> +       do {
> +               rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> +                               FSCTL_DFS_GET_REFERRALS, true /* is_fsctl */,
> +                               (char *)dfs_req, dfs_req_size,
> +                               (char **)&dfs_rsp, &dfs_rsp_size);
> +       } while (rc == -EAGAIN);
> +
> +       if (rc) {
> +               cifs_dbg(VFS, "ioctl error in smb2_get_dfs_refer rc=%d\n", rc);
> +               goto out;
> +       }
> +
> +       rc = parse_dfs_referrals(dfs_rsp, dfs_rsp_size,
> +                                num_of_nodes, target_nodes,
> +                                nls_codepage, remap, search_name,
> +                                true /* is_unicode */);
> +       if (rc) {
> +               cifs_dbg(VFS, "parse error in smb2_get_dfs_refer rc=%d\n", rc);
> +               goto out;
> +       }
> +
> + out:
> +       kfree(utf16_path);
> +       kfree(dfs_req);
> +       kfree(dfs_rsp);
> +       return rc;
> +}
>  #define SMB2_SYMLINK_STRUCT_SIZE \
>         (sizeof(struct smb2_err_rsp) - 1 + sizeof(struct smb2_symlink_err_rsp))
>
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index c03b252..18700fd 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -695,6 +695,14 @@ struct fsctl_get_integrity_information_rsp {
>  /* Integrity flags for above */
>  #define FSCTL_INTEGRITY_FLAG_CHECKSUM_ENFORCEMENT_OFF  0x00000001
>
> +/* See MS-DFSC 2.2.2 */
> +struct fsctl_get_dfs_referral_req {
> +       __le16 MaxReferralLevel;
> +       __u8 RequestFileName[];
> +} __packed;
> +
> +/* DFS response is struct get_dfs_refer_rsp */
> +
>  /* See MS-SMB2 2.2.31.3 */
>  struct network_resiliency_req {
>         __le32 Timeout;
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best regards,
Pavel Shilovsky

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Aurélien Aptel]

Hi Pavel,

Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Accessing the tcon_list should be protected by spin_locks and also a
> reference to the tcon should be acquired before using it safely.

I've noticed we have the cifs_tcp_ses_lock but also a per-session
session_mutex. I'm guessing we want cifs_tcp_ses_lock here.

By acquiring a reference you mean incrementing/decrementing tc_count. Do
we need to protect this inc/dec operation with cifs_tcp_ses_lock too?

> Also you can extend SMB2_ioctl argument list to allow passing ipc_tid.
> In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if
> tcon is passed -use it, otherwise - use ipc_tid.
>
> In this case you can fallback to ipc_tid if no tcon is found in the list.
>

SMB2_ioctl() uses the tcon for many things. I don't see how we can just
use ipc_tid number if we don't pass a tcon.

Or do you mean: add a use_ipc bool to SMB2_ioctl arg list and use the tcon as
usual except overwrite the Tid field with ses->ipc_tid before sending?

Thanks,

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Pavel Shilovsky]

2017-02-24 7:25 GMT-08:00 Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Hi Pavel,
>
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> Accessing the tcon_list should be protected by spin_locks and also a
>> reference to the tcon should be acquired before using it safely.
>
> I've noticed we have the cifs_tcp_ses_lock but also a per-session
> session_mutex. I'm guessing we want cifs_tcp_ses_lock here.
>
> By acquiring a reference you mean incrementing/decrementing tc_count. Do
> we need to protect this inc/dec operation with cifs_tcp_ses_lock too?

Yes.

>
>> Also you can extend SMB2_ioctl argument list to allow passing ipc_tid.
>> In this case SMB2_ioctl() can choose what to use - ipc_tid or tcon; if
>> tcon is passed -use it, otherwise - use ipc_tid.
>>
>> In this case you can fallback to ipc_tid if no tcon is found in the list.
>>
>
> SMB2_ioctl() uses the tcon for many things. I don't see how we can just
> use ipc_tid number if we don't pass a tcon.
>
> Or do you mean: add a use_ipc bool to SMB2_ioctl arg list and use the tcon as
> usual except overwrite the Tid field with ses->ipc_tid before sending?

We can pass both arguments: tcon and ipc_tid. If the 1st is specified
SMB_ioctl uses it, otherwise - use ipc_tid.

Since we call get_dfs_refer() once we get ipc_tid, may be it is worth
to try ipc_tid at first and then, if EAGAIN is returned (probably due
to reconnection that makes ipc_tid equal 0), use the 1st tcon from
list for the session. This will also workaround the bug in Samba.

>
> Thanks,
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)



-- 
Best regards,
Pavel Shilovsky

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Aurélien Aptel]

Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org> writes:
> What I'm proposing is to add a bool flag use_ipc to SMB2_ioctl. And do
> something like this:
>
>     if use_ipc:
>         if tcon->ses->ipc_tid == 0:
>             return -ENOTCONN
>         else:
>             req->tid = tcon->ses->ipc_tid
>
> Now in smb2_get_dfs_referral can first try to call it with
> use_ipc=true and if that fails with ENOTCONN try again with false.

I went ahead and implemented this along with small improvements in a v3
patchset. I'm still open to rework on this if can understand better what
you were proposing.

Cheers,

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH v2 3/6] CIFS: implement get_dfs_refer for SMB2+

[Pavel Shilovsky]

2017-02-27 6:17 GMT-08:00 Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> We can pass both arguments: tcon and ipc_tid. If the 1st is specified
>> SMB_ioctl uses it, otherwise - use ipc_tid.
>
> I still don't get it. When tcon is not specified (I'm assuming that
> means passing NULL) and we just have ipc_tid (an int) we cannot:
> - access the struct cifs_ses*
> - access the struct TCP_Server_Info*
> - check if encryption is required
> - call any of the functions that requires the session or the server
>   struct (i.e. all the functions that do the real work).

Yes, we still need to pass "ses" argument which will create a whole
mess. But we probably need to clean this anyway because the number of
argument are getting too big.

>
>> Since we call get_dfs_refer() once we get ipc_tid, may be it is worth
>> to try ipc_tid at first and then, if EAGAIN is returned (probably due
>> to reconnection that makes ipc_tid equal 0), use the 1st tcon from
>> list for the session. This will also workaround the bug in Samba.
>
> What I'm proposing is to add a bool flag use_ipc to SMB2_ioctl. And do
> something like this:
>
>     if use_ipc:
>         if tcon->ses->ipc_tid == 0:
>             return -ENOTCONN
>         else:
>             req->tid = tcon->ses->ipc_tid
>
> Now in smb2_get_dfs_referral can first try to call it with
> use_ipc=true and if that fails with ENOTCONN try again with false.

This also works although it doesn't allow to call SMB2_ioctl() without
having at least one tcon.

-- 
Best regards,
Pavel Shilovsky