[PATCH v1 0/3] Make IPC a regular tcon and fix SMB2 domain-based DFS

[PATCH v1 0/3] Make IPC a regular tcon and fix SMB2 domain-based DFS

[Aurelien Aptel]

Hi,

The DFS code for smb2 doesn't work [1] when cifs.ko connects to a
domain-based DFS where you have an extra level on indirection (so 3
machines: AD, nameserver and the final storage).

* In SMB1 code, the DFS request is made without the tcon object. It works
  directly on the session.
* In SMB2 code, we use SMB2_ioctl() which works on a tcon.

For domain-based DFS UNC \\DOMAIN\namespace\link, cifs.ko first tries to
connect to \\DOMAIN\namespace. DOMAIN doesn't have a "namespace" share
so no valid tcon object is created.

* SMB1 doesn't care as it's working on the session object and just use
  the ses->ipc_tid number to make the DFS request.
* SMB2 code on the other hand fails as SMB2_ioctl() needs a tcon
  object (even if you pass use_ipc=true) because it uses the tcon you
  pass and simply overwrites the packet tid with ipc_tid. There is no
  valid tcon object at that time since DOMAIN rejected the "namespace"
  tcon request.

So to solve this I thought I would get rid of ses->ipc_tid and make a
real IPC tcon, that would be added to the session tcon_list and be
managed like any other tcon (almost).

This patch series cleans some definitions and makes the IPC connection
a regular tcon owned by the session. It removes the use_ipc param from
SMB2_ioct() (you can pass ses->tcon_ipc as the tcon param).

I've made the choice of having the IPC tcon completely associated to
the session instead of any other tcon. Its refcount is not really used
as it's always created as soon as we have a valid session and always
destroy when the session is destroyed and no mount points directly
refer to it.

All patches compile when applied sequentially, no warnings with
checkpatch.pl, no warnings with make C=1.

I've tested this against Windows Server 2016 vms with smb1, 2, 3 (with
and without encryption) and it seems to be working properly.

I've also quickly tested the reconnection code path but it probably
needs some work still (making sure DFS works after a reconnection) as
there are multiple ways to trigger the reconnection (I've been using
qemu monitor "set_link xyz off" which is analogous to unplugging the
network cable IIUC).

Please take a look, review, test :)

1: https://bugzilla.samba.org/show_bug.cgi?id=12917

Aurelien Aptel (3):
  CIFS: set SERVER_NAME_LENGTH to serverName actual size
  CIFS: make IPC a regular tcon
  CIFS: use tcon_ipc instead of use_ipc parameter from SMB2_ioctl

 fs/cifs/cifsglob.h  |  14 ++---
 fs/cifs/cifssmb.c   |   5 +-
 fs/cifs/connect.c   | 159 ++++++++++++++++++++++++++++++++++++++++------------
 fs/cifs/inode.c     |   2 +-
 fs/cifs/smb2file.c  |   2 +-
 fs/cifs/smb2ops.c   |  50 +++++++----------
 fs/cifs/smb2pdu.c   |  36 ++----------
 fs/cifs/smb2proto.h |   3 +-
 8 files changed, 160 insertions(+), 111 deletions(-)

-- 
2.12.3

[PATCH v1 1/3] CIFS: set SERVER_NAME_LENGTH to serverName actual size

[Aurelien Aptel]

The maximum length of an ipv6 string representation is defined in
INET6_ADDRSTRLEN as 45+1 for null but lets keep what we know works.

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

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index b16583594d1a..eca386d8dcfb 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -64,8 +64,8 @@
 #define RFC1001_NAME_LEN 15
 #define RFC1001_NAME_LEN_WITH_NULL (RFC1001_NAME_LEN + 1)
 
-/* currently length of NIP6_FMT */
-#define SERVER_NAME_LENGTH 40
+/* maximum length of ip addr as a string (including ipv6 and sctp) */
+#define SERVER_NAME_LENGTH 80
 #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
 
 /* echo interval in seconds */
@@ -835,8 +835,7 @@ struct cifs_ses {
 	kuid_t linux_uid;	/* overriding owner of files on the mount */
 	kuid_t cred_uid;	/* owner of credentials */
 	unsigned int capabilities;
-	char serverName[SERVER_NAME_LEN_WITH_NULL * 2];	/* BB make bigger for
-				TCP names - will ipv6 and sctp addresses fit? */
+	char serverName[SERVER_NAME_LEN_WITH_NULL];
 	char *user_name;	/* must not be null except during init of sess
 				   and after mount option parsing we fill it */
 	char *domainName;
-- 
2.12.3

[PATCH v1 2/3] CIFS: make IPC a regular tcon

[Aurelien Aptel]

* Remove ses->ipc_tid.
* Make IPC$ regular tcon by having it stored in the session tcon list.
* Add a direct pointer to it in ses->tcon_ipc.
* Distinguish PIPE tcon from IPC tcon by adding a tcon->pipe flag. All
  IPC tcons are pipes but not all pipes are IPC.
* All TreeConnect functions now cannot take a NULL tcon object.

The IPC tcon has the same lifetime as the session it belongs to. It is
created when the session is created and destroyed when the session is
destroyed.

Since no mounts directly refer to the IPC tcon, its refcount should
always be 0. Thus we make sure cifs_put_tcon() skips it.

If the mount request resulting in a new session being created requires
encryption, try to require it too for IPC.

NOTE TO BACKPORTER: the previous SERVER_NAME_LENGTH change MUST be
included with this.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifsglob.h |   7 ++-
 fs/cifs/cifssmb.c  |   5 +-
 fs/cifs/connect.c  | 159 ++++++++++++++++++++++++++++++++++++++++-------------
 fs/cifs/inode.c    |   2 +-
 fs/cifs/smb2pdu.c  |  32 ++---------
 5 files changed, 134 insertions(+), 71 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index eca386d8dcfb..804ec79af5e1 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -822,12 +822,12 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
 struct cifs_ses {
 	struct list_head smb_ses_list;
 	struct list_head tcon_list;
+	struct cifs_tcon *tcon_ipc;
 	struct mutex session_mutex;
 	struct TCP_Server_Info *server;	/* pointer to server info */
 	int ses_count;		/* reference counter */
 	enum statusEnum status;
 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
-	__u32 ipc_tid;		/* special tid for connection to IPC share */
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
 	char *serverDomain;	/* security realm of server */
@@ -930,7 +930,9 @@ struct cifs_tcon {
 	FILE_SYSTEM_DEVICE_INFO fsDevInfo;
 	FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
 	FILE_SYSTEM_UNIX_INFO fsUnixInfo;
-	bool ipc:1;		/* set if connection to IPC$ eg for RPC/PIPES */
+	bool ipc:1;   /* set if connection to IPC$ share (always also pipe) */
+	bool pipe:1;  /* set if connection to pipe share */
+	bool print:1; /* set if connection to printer share */
 	bool retry:1;
 	bool nocase:1;
 	bool seal:1;      /* transport encryption for this mounted share */
@@ -943,7 +945,6 @@ struct cifs_tcon {
 	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 */
-	bool print:1;		/* set if connection to printer share */
 	__le32 capabilities;
 	__u32 share_flags;
 	__u32 maximal_access;
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 35dc5bf01ee2..2f8dea905ef9 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4822,8 +4822,9 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	*target_nodes = NULL;
 
 	cifs_dbg(FYI, "In GetDFSRefer the path %s\n", search_name);
-	if (ses == NULL)
+	if (ses == NULL || ses->tcon_ipc == NULL)
 		return -ENODEV;
+
 getDFSRetry:
 	rc = smb_init(SMB_COM_TRANSACTION2, 15, NULL, (void **) &pSMB,
 		      (void **) &pSMBr);
@@ -4833,7 +4834,7 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	/* server pointer checked in called function,
 	but should never be null here anyway */
 	pSMB->hdr.Mid = get_next_mid(ses->server);
-	pSMB->hdr.Tid = ses->ipc_tid;
+	pSMB->hdr.Tid = ses->tcon_ipc->tid;
 	pSMB->hdr.Uid = ses->Suid;
 	if (ses->capabilities & CAP_STATUS32)
 		pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 0bfc2280436d..5ae393fe0c5d 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -353,7 +353,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	list_for_each(tmp, &server->smb_ses_list) {
 		ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
 		ses->need_reconnect = true;
-		ses->ipc_tid = 0;
 		list_for_each(tmp2, &ses->tcon_list) {
 			tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
 			tcon->need_reconnect = true;
@@ -2381,6 +2380,104 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
 	return 1;
 }
 
+/**
+ * cifs_setup_ipc - helper to setup the IPC tcon for the session
+ *
+ * A new IPC connection is made and added to the session tcon list
+ * (also accessible through ses->tcon_ipc). The IPC tcon has the same
+ * lifetime as the session.
+ */
+static int
+cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon;
+	struct nls_table *nls_codepage;
+	char unc[SERVER_NAME_LENGTH + sizeof("//x/IPC$")] = {0};
+	bool seal = false;
+
+	/*
+	 * If the mount request that resulted in the creation of the
+	 * session requires encryption, force IPC to be encrypted too.
+	 */
+	if (volume_info->seal) {
+		if (ses->server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)
+			seal = true;
+		else {
+			cifs_dbg(VFS,
+				 "IPC: server doesn't support encryption\n");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	tcon = tconInfoAlloc();
+	if (tcon == NULL)
+		return -ENOMEM;
+
+	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
+
+	/* cannot fail */
+	nls_codepage = load_nls_default();
+
+	xid = get_xid();
+	tcon->ses = ses;
+	tcon->ipc = true;
+	tcon->seal = seal;
+	rc = ses->server->ops->tree_connect(xid, ses, unc, tcon, nls_codepage);
+	free_xid(xid);
+
+	if (rc) {
+		cifs_dbg(VFS, "failed to connect to IPC (rc=%d)\n", rc);
+		tconInfoFree(tcon);
+		goto out;
+	}
+
+	cifs_dbg(FYI, "IPC tcon rc = %d ipc tid = %d\n", rc, tcon->tid);
+
+	spin_lock(&cifs_tcp_ses_lock);
+	list_add(&tcon->tcon_list, &ses->tcon_list);
+	ses->tcon_ipc = tcon;
+	spin_unlock(&cifs_tcp_ses_lock);
+
+out:
+	unload_nls(nls_codepage);
+	return rc;
+}
+
+/**
+ * cifs_free_ipc - helper to release the session IPC tcon
+ *
+ * Needs to be called everytime a session is destroyed
+ */
+static int
+cifs_free_ipc(struct cifs_ses *ses)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon;
+
+	spin_lock(&cifs_tcp_ses_lock);
+	tcon = ses->tcon_ipc;
+	if (tcon == NULL) {
+		spin_unlock(&cifs_tcp_ses_lock);
+		return 0;
+	}
+	list_del_init(&tcon->tcon_list);
+	spin_unlock(&cifs_tcp_ses_lock);
+
+	if (ses->server->ops->tree_disconnect) {
+		xid = get_xid();
+		rc = ses->server->ops->tree_disconnect(xid, tcon);
+		free_xid(xid);
+	}
+
+	if (rc)
+		cifs_dbg(FYI, "failed to disconnect IPC tcon (rc=%d)\n", rc);
+
+	tconInfoFree(tcon);
+	ses->tcon_ipc = NULL;
+	return rc;
+}
+
 static struct cifs_ses *
 cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
 {
@@ -2421,6 +2518,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
 		ses->status = CifsExiting;
 	spin_unlock(&cifs_tcp_ses_lock);
 
+	cifs_free_ipc(ses);
+
 	if (ses->status == CifsExiting && server->ops->logoff) {
 		xid = get_xid();
 		rc = server->ops->logoff(xid, ses);
@@ -2665,6 +2764,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	free_xid(xid);
+
+	cifs_setup_ipc(ses, volume_info);
+
 	return ses;
 
 get_ses_fail:
@@ -2709,8 +2811,16 @@ void
 cifs_put_tcon(struct cifs_tcon *tcon)
 {
 	unsigned int xid;
-	struct cifs_ses *ses = tcon->ses;
+	struct cifs_ses *ses;
+
+	/*
+	 * IPC tcon share the lifetime of their session and are
+	 * destroyed in the session put function
+	 */
+	if (tcon == NULL || tcon->ipc)
+		return;
 
+	ses = tcon->ses;
 	cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
 	spin_lock(&cifs_tcp_ses_lock);
 	if (--tcon->tc_count > 0) {
@@ -2986,39 +3096,17 @@ get_dfs_path(const unsigned int xid, struct cifs_ses *ses, const char *old_path,
 	     const struct nls_table *nls_codepage, unsigned int *num_referrals,
 	     struct dfs_info3_param **referrals, int remap)
 {
-	char *temp_unc;
 	int rc = 0;
 
-	if (!ses->server->ops->tree_connect || !ses->server->ops->get_dfs_refer)
+	if (!ses->server->ops->get_dfs_refer)
 		return -ENOSYS;
 
 	*num_referrals = 0;
 	*referrals = NULL;
 
-	if (ses->ipc_tid == 0) {
-		temp_unc = kmalloc(2 /* for slashes */ +
-			strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
-				+ 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
-		if (temp_unc == NULL)
-			return -ENOMEM;
-		temp_unc[0] = '\\';
-		temp_unc[1] = '\\';
-		strcpy(temp_unc + 2, ses->serverName);
-		strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
-		rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
-						    nls_codepage);
-		cifs_dbg(FYI, "Tcon rc = %d ipc_tid = %d\n", rc, ses->ipc_tid);
-		kfree(temp_unc);
-	}
-	if (rc == 0)
-		rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
-						     referrals, num_referrals,
-						     nls_codepage, remap);
-	/*
-	 * BB - map targetUNCs to dfs_info3 structures, here or in
-	 * ses->server->ops->get_dfs_refer.
-	 */
-
+	rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
+					     referrals, num_referrals,
+					     nls_codepage, remap);
 	return rc;
 }
 
@@ -3783,7 +3871,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 		tcon->unix_ext = 0; /* server does not support them */
 
 	/* do not care if a following call succeed - informational */
-	if (!tcon->ipc && server->ops->qfs_tcon)
+	if (!tcon->pipe && server->ops->qfs_tcon)
 		server->ops->qfs_tcon(xid, tcon);
 
 	cifs_sb->wsize = server->ops->negotiate_wsize(tcon, volume_info);
@@ -3913,8 +4001,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 }
 
 /*
- * Issue a TREE_CONNECT request. Note that for IPC$ shares, that the tcon
- * pointer may be NULL.
+ * Issue a TREE_CONNECT request.
  */
 int
 CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
@@ -3950,7 +4037,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 	pSMB->AndXCommand = 0xFF;
 	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
 	bcc_ptr = &pSMB->Password[0];
-	if (!tcon || (ses->server->sec_mode & SECMODE_USER)) {
+	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
 		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
 		*bcc_ptr = 0; /* password is null byte */
 		bcc_ptr++;              /* skip password */
@@ -4022,7 +4109,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			 0);
 
 	/* above now done in SendReceive */
-	if ((rc == 0) && (tcon != NULL)) {
+	if (rc == 0) {
 		bool is_unicode;
 
 		tcon->tidStatus = CifsGood;
@@ -4042,7 +4129,8 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
 			    (bcc_ptr[2] == 'C')) {
 				cifs_dbg(FYI, "IPC connection\n");
-				tcon->ipc = 1;
+				tcon->ipc = true;
+				tcon->pipe = true;
 			}
 		} else if (length == 2) {
 			if ((bcc_ptr[0] == 'A') && (bcc_ptr[1] == ':')) {
@@ -4069,9 +4157,6 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		else
 			tcon->Flags = 0;
 		cifs_dbg(FYI, "Tcon flags: 0x%x\n", tcon->Flags);
-	} else if ((rc == 0) && tcon == NULL) {
-		/* all we need to save for IPC$ connection */
-		ses->ipc_tid = smb_buffer_response->Tid;
 	}
 
 	cifs_buf_release(smb_buffer);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ecb99079363a..8f9a8cc7cc62 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1049,7 +1049,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 	tcon->resource_id = CIFS_I(inode)->uniqueid;
 #endif
 
-	if (rc && tcon->ipc) {
+	if (rc && tcon->pipe) {
 		cifs_dbg(FYI, "ipc connection - fake read inode\n");
 		spin_lock(&inode->i_lock);
 		inode->i_mode |= S_IFDIR;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 01346b8b6edb..a5c94f8308eb 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1283,8 +1283,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	}
 
 	/* SMB2 TREE_CONNECT request must be called with TreeId == 0 */
-	if (tcon)
-		tcon->tid = 0;
+	tcon->tid = 0;
 
 	rc = small_smb2_init(SMB2_TREE_CONNECT, tcon, (void **) &req);
 	if (rc) {
@@ -1292,15 +1291,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		return rc;
 	}
 
-	if (tcon == NULL) {
-		if ((ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
-			flags |= CIFS_TRANSFORM_REQ;
-
-		/* since no tcon, smb2_init can not do this, so do here */
-		req->hdr.sync_hdr.SessionId = ses->Suid;
-		if (ses->server->sign)
-			req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
-	} else if (encryption_required(tcon))
+	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
 	iov[0].iov_base = (char *)req;
@@ -1328,21 +1319,16 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		goto tcon_error_exit;
 	}
 
-	if (tcon == NULL) {
-		ses->ipc_tid = rsp->hdr.sync_hdr.TreeId;
-		goto tcon_exit;
-	}
-
 	switch (rsp->ShareType) {
 	case SMB2_SHARE_TYPE_DISK:
 		cifs_dbg(FYI, "connection to disk share\n");
 		break;
 	case SMB2_SHARE_TYPE_PIPE:
-		tcon->ipc = true;
+		tcon->pipe = true;
 		cifs_dbg(FYI, "connection to pipe share\n");
 		break;
 	case SMB2_SHARE_TYPE_PRINT:
-		tcon->ipc = true;
+		tcon->print = true;
 		cifs_dbg(FYI, "connection to printer\n");
 		break;
 	default:
@@ -1913,16 +1899,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	if (rc)
 		return rc;
 
-	if (use_ipc) {
-		if (ses->ipc_tid == 0) {
-			cifs_small_buf_release(req);
-			return -ENOTCONN;
-		}
-
-		cifs_dbg(FYI, "replacing tid 0x%x with IPC tid 0x%x\n",
-			 req->hdr.sync_hdr.TreeId, ses->ipc_tid);
-		req->hdr.sync_hdr.TreeId = ses->ipc_tid;
-	}
 	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
-- 
2.12.3

RE: [PATCH v1 2/3] CIFS: make IPC a regular tcon

[Pavel Shilovskiy]

2018-01-17 9:21 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> * Remove ses->ipc_tid.
> * Make IPC$ regular tcon by having it stored in the session tcon list.

Thank you for doing this, I suspected that we would end up with something like it anyway.

> * Add a direct pointer to it in ses->tcon_ipc.
> * Distinguish PIPE tcon from IPC tcon by adding a tcon->pipe flag. All
>   IPC tcons are pipes but not all pipes are IPC.
> * All TreeConnect functions now cannot take a NULL tcon object.
>
> The IPC tcon has the same lifetime as the session it belongs to. It is
> created when the session is created and destroyed when the session is
> destroyed.
>
> Since no mounts directly refer to the IPC tcon, its refcount should
> always be 0. Thus we make sure cifs_put_tcon() skips it.
>
> If the mount request resulting in a new session being created requires
> encryption, try to require it too for IPC.
>
> NOTE TO BACKPORTER: the previous SERVER_NAME_LENGTH change MUST be
> included with this.

The 1st patch is small, so let's merge both patches that would make backporter's life easier.

>
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |   7 ++-
>  fs/cifs/cifssmb.c  |   5 +-
>  fs/cifs/connect.c  | 159 ++++++++++++++++++++++++++++++++++++++++-------------
>  fs/cifs/inode.c    |   2 +-
>  fs/cifs/smb2pdu.c  |  32 ++---------
>  5 files changed, 134 insertions(+), 71 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index eca386d8dcfb..804ec79af5e1 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -822,12 +822,12 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
>  struct cifs_ses {
>         struct list_head smb_ses_list;
>         struct list_head tcon_list;
> +       struct cifs_tcon *tcon_ipc;
>         struct mutex session_mutex;
>         struct TCP_Server_Info *server; /* pointer to server info */
>         int ses_count;          /* reference counter */
>         enum statusEnum status;
>         unsigned overrideSecFlg;  /* if non-zero override global sec flags */
> -       __u32 ipc_tid;          /* special tid for connection to IPC share */
>         char *serverOS;         /* name of operating system underlying server */
>         char *serverNOS;        /* name of network operating system of server */
>         char *serverDomain;     /* security realm of server */
> @@ -930,7 +930,9 @@ struct cifs_tcon {
>         FILE_SYSTEM_DEVICE_INFO fsDevInfo;
>         FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
>         FILE_SYSTEM_UNIX_INFO fsUnixInfo;
> -       bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
> +       bool ipc:1;   /* set if connection to IPC$ share (always also pipe) */
> +       bool pipe:1;  /* set if connection to pipe share */
> +       bool print:1; /* set if connection to printer share */
>         bool retry:1;
>         bool nocase:1;
>         bool seal:1;      /* transport encryption for this mounted share */
> @@ -943,7 +945,6 @@ struct cifs_tcon {
>         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 */
> -       bool print:1;           /* set if connection to printer share */
>         __le32 capabilities;
>         __u32 share_flags;
>         __u32 maximal_access;
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 35dc5bf01ee2..2f8dea905ef9 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4822,8 +4822,9 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>         *target_nodes = NULL;
>
>         cifs_dbg(FYI, "In GetDFSRefer the path %s\n", search_name);
> -       if (ses == NULL)
> +       if (ses == NULL || ses->tcon_ipc == NULL)
>                 return -ENODEV;
> +
>  getDFSRetry:
>         rc = smb_init(SMB_COM_TRANSACTION2, 15, NULL, (void **) &pSMB,
>                       (void **) &pSMBr);
> @@ -4833,7 +4834,7 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>         /* server pointer checked in called function,
>         but should never be null here anyway */
>         pSMB->hdr.Mid = get_next_mid(ses->server);
> -       pSMB->hdr.Tid = ses->ipc_tid;
> +       pSMB->hdr.Tid = ses->tcon_ipc->tid;
>         pSMB->hdr.Uid = ses->Suid;
>         if (ses->capabilities & CAP_STATUS32)
>                 pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 0bfc2280436d..5ae393fe0c5d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -353,7 +353,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
>         list_for_each(tmp, &server->smb_ses_list) {
>                 ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
>                 ses->need_reconnect = true;
> -               ses->ipc_tid = 0;
>                 list_for_each(tmp2, &ses->tcon_list) {
>                         tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
>                         tcon->need_reconnect = true;
> @@ -2381,6 +2380,104 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
>         return 1;
>  }
>
> +/**
> + * cifs_setup_ipc - helper to setup the IPC tcon for the session
> + *
> + * A new IPC connection is made and added to the session tcon list
> + * (also accessible through ses->tcon_ipc). The IPC tcon has the same
> + * lifetime as the session.
> + */
> +static int
> +cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
> +{
> +       int rc = 0, xid;
> +       struct cifs_tcon *tcon;
> +       struct nls_table *nls_codepage;
> +       char unc[SERVER_NAME_LENGTH + sizeof("//x/IPC$")] = {0};
> +       bool seal = false;
> +
> +       /*
> +        * If the mount request that resulted in the creation of the
> +        * session requires encryption, force IPC to be encrypted too.
> +        */
> +       if (volume_info->seal) {
> +               if (ses->server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)
> +                       seal = true;
> +               else {
> +                       cifs_dbg(VFS,
> +                                "IPC: server doesn't support encryption\n");
> +                       return -EOPNOTSUPP;
> +               }
> +       }
> +
> +       tcon = tconInfoAlloc();
> +       if (tcon == NULL)
> +               return -ENOMEM;
> +
> +       snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
> +
> +       /* cannot fail */
> +       nls_codepage = load_nls_default();
> +
> +       xid = get_xid();
> +       tcon->ses = ses;
> +       tcon->ipc = true;
> +       tcon->seal = seal;
> +       rc = ses->server->ops->tree_connect(xid, ses, unc, tcon, nls_codepage);
> +       free_xid(xid);
> +
> +       if (rc) {
> +               cifs_dbg(VFS, "failed to connect to IPC (rc=%d)\n", rc);
> +               tconInfoFree(tcon);
> +               goto out;
> +       }
> +
> +       cifs_dbg(FYI, "IPC tcon rc = %d ipc tid = %d\n", rc, tcon->tid);
> +
> +       spin_lock(&cifs_tcp_ses_lock);
> +       list_add(&tcon->tcon_list, &ses->tcon_list);
> +       ses->tcon_ipc = tcon;
> +       spin_unlock(&cifs_tcp_ses_lock);
> +
> +out:
> +       unload_nls(nls_codepage);
> +       return rc;
> +}
> +
> +/**
> + * cifs_free_ipc - helper to release the session IPC tcon
> + *
> + * Needs to be called everytime a session is destroyed
> + */
> +static int
> +cifs_free_ipc(struct cifs_ses *ses)
> +{
> +       int rc = 0, xid;
> +       struct cifs_tcon *tcon;
> +
> +       spin_lock(&cifs_tcp_ses_lock);
> +       tcon = ses->tcon_ipc;
> +       if (tcon == NULL) {
> +               spin_unlock(&cifs_tcp_ses_lock);
> +               return 0;
> +       }
> +       list_del_init(&tcon->tcon_list);
> +       spin_unlock(&cifs_tcp_ses_lock);
> +
> +       if (ses->server->ops->tree_disconnect) {
> +               xid = get_xid();
> +               rc = ses->server->ops->tree_disconnect(xid, tcon);
> +               free_xid(xid);
> +       }
> +
> +       if (rc)
> +               cifs_dbg(FYI, "failed to disconnect IPC tcon (rc=%d)\n", rc);
> +
> +       tconInfoFree(tcon);
> +       ses->tcon_ipc = NULL;

Minor: it should probably work this way, but since we set ses->tcon_ipc under a spinlock in cifs_setup_ipc(), it would be better to do something similar here.

> +       return rc;
> +}
> +
>  static struct cifs_ses *
>  cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
>  {
> @@ -2421,6 +2518,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>                 ses->status = CifsExiting;
>         spin_unlock(&cifs_tcp_ses_lock);
>
> +       cifs_free_ipc(ses);
> +
>         if (ses->status == CifsExiting && server->ops->logoff) {
>                 xid = get_xid();
>                 rc = server->ops->logoff(xid, ses);
> @@ -2665,6 +2764,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>         spin_unlock(&cifs_tcp_ses_lock);
>
>         free_xid(xid);
> +
> +       cifs_setup_ipc(ses, volume_info);
> +
>         return ses;
>
>  get_ses_fail:
> @@ -2709,8 +2811,16 @@ void
>  cifs_put_tcon(struct cifs_tcon *tcon)
>  {
>         unsigned int xid;
> -       struct cifs_ses *ses = tcon->ses;
> +       struct cifs_ses *ses;
> +
> +       /*
> +        * IPC tcon share the lifetime of their session and are
> +        * destroyed in the session put function
> +        */
> +       if (tcon == NULL || tcon->ipc)
> +               return;
>
> +       ses = tcon->ses;
>         cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
>         spin_lock(&cifs_tcp_ses_lock);
>         if (--tcon->tc_count > 0) {
> @@ -2986,39 +3096,17 @@ get_dfs_path(const unsigned int xid, struct cifs_ses *ses, const char *old_path,
>              const struct nls_table *nls_codepage, unsigned int *num_referrals,
>              struct dfs_info3_param **referrals, int remap)
>  {
> -       char *temp_unc;
>         int rc = 0;
>
> -       if (!ses->server->ops->tree_connect || !ses->server->ops->get_dfs_refer)
> +       if (!ses->server->ops->get_dfs_refer)
>                 return -ENOSYS;
>
>         *num_referrals = 0;
>         *referrals = NULL;
>
> -       if (ses->ipc_tid == 0) {
> -               temp_unc = kmalloc(2 /* for slashes */ +
> -                       strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
> -                               + 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
> -               if (temp_unc == NULL)
> -                       return -ENOMEM;
> -               temp_unc[0] = '\\';
> -               temp_unc[1] = '\\';
> -               strcpy(temp_unc + 2, ses->serverName);
> -               strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
> -               rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
> -                                                   nls_codepage);
> -               cifs_dbg(FYI, "Tcon rc = %d ipc_tid = %d\n", rc, ses->ipc_tid);
> -               kfree(temp_unc);
> -       }
> -       if (rc == 0)
> -               rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
> -                                                    referrals, num_referrals,
> -                                                    nls_codepage, remap);
> -       /*
> -        * BB - map targetUNCs to dfs_info3 structures, here or in
> -        * ses->server->ops->get_dfs_refer.
> -        */

Is this no longer needed?

> -
> +       rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
> +                                            referrals, num_referrals,
> +                                            nls_codepage, remap);
>         return rc;
>  }
>
> @@ -3783,7 +3871,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
>                 tcon->unix_ext = 0; /* server does not support them */
>
>         /* do not care if a following call succeed - informational */
> -       if (!tcon->ipc && server->ops->qfs_tcon)
> +       if (!tcon->pipe && server->ops->qfs_tcon)
>                 server->ops->qfs_tcon(xid, tcon);
>
>         cifs_sb->wsize = server->ops->negotiate_wsize(tcon, volume_info);
> @@ -3913,8 +4001,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
>  }
>
>  /*
> - * Issue a TREE_CONNECT request. Note that for IPC$ shares, that the tcon
> - * pointer may be NULL.
> + * Issue a TREE_CONNECT request.
>   */
>  int
>  CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> @@ -3950,7 +4037,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>         pSMB->AndXCommand = 0xFF;
>         pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
>         bcc_ptr = &pSMB->Password[0];
> -       if (!tcon || (ses->server->sec_mode & SECMODE_USER)) {
> +       if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
>                 pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
>                 *bcc_ptr = 0; /* password is null byte */
>                 bcc_ptr++;              /* skip password */
> @@ -4022,7 +4109,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                          0);
>
>         /* above now done in SendReceive */
> -       if ((rc == 0) && (tcon != NULL)) {
> +       if (rc == 0) {
>                 bool is_unicode;
>
>                 tcon->tidStatus = CifsGood;
> @@ -4042,7 +4129,8 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                         if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
>                             (bcc_ptr[2] == 'C')) {
>                                 cifs_dbg(FYI, "IPC connection\n");
> -                               tcon->ipc = 1;
> +                               tcon->ipc = true;
> +                               tcon->pipe = true;
>                         }
>                 } else if (length == 2) {
>                         if ((bcc_ptr[0] == 'A') && (bcc_ptr[1] == ':')) {
> @@ -4069,9 +4157,6 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                 else
>                         tcon->Flags = 0;
>                 cifs_dbg(FYI, "Tcon flags: 0x%x\n", tcon->Flags);
> -       } else if ((rc == 0) && tcon == NULL) {
> -               /* all we need to save for IPC$ connection */
> -               ses->ipc_tid = smb_buffer_response->Tid;
>         }
>
>         cifs_buf_release(smb_buffer);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index ecb99079363a..8f9a8cc7cc62 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1049,7 +1049,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
>         tcon->resource_id = CIFS_I(inode)->uniqueid;
>  #endif
>
> -       if (rc && tcon->ipc) {
> +       if (rc && tcon->pipe) {
>                 cifs_dbg(FYI, "ipc connection - fake read inode\n");
>                 spin_lock(&inode->i_lock);
>                 inode->i_mode |= S_IFDIR;
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 01346b8b6edb..a5c94f8308eb 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1283,8 +1283,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>         }
>
>         /* SMB2 TREE_CONNECT request must be called with TreeId == 0 */
> -       if (tcon)
> -               tcon->tid = 0;
> +       tcon->tid = 0;
>
>         rc = small_smb2_init(SMB2_TREE_CONNECT, tcon, (void **) &req);
>         if (rc) {
> @@ -1292,15 +1291,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>                 return rc;
>         }
>
> -       if (tcon == NULL) {
> -               if ((ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
> -                       flags |= CIFS_TRANSFORM_REQ;
> -
> -               /* since no tcon, smb2_init can not do this, so do here */
> -               req->hdr.sync_hdr.SessionId = ses->Suid;
> -               if (ses->server->sign)
> -                       req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
> -       } else if (encryption_required(tcon))
> +       if (encryption_required(tcon))
>                 flags |= CIFS_TRANSFORM_REQ;
>
>         iov[0].iov_base = (char *)req;
> @@ -1328,21 +1319,16 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>                 goto tcon_error_exit;
>         }
>
> -       if (tcon == NULL) {
> -               ses->ipc_tid = rsp->hdr.sync_hdr.TreeId;
> -               goto tcon_exit;
> -       }
> -
>         switch (rsp->ShareType) {
>         case SMB2_SHARE_TYPE_DISK:
>                 cifs_dbg(FYI, "connection to disk share\n");
>                 break;
>         case SMB2_SHARE_TYPE_PIPE:
> -               tcon->ipc = true;
> +               tcon->pipe = true;
>                 cifs_dbg(FYI, "connection to pipe share\n");
>                 break;
>         case SMB2_SHARE_TYPE_PRINT:
> -               tcon->ipc = true;
> +               tcon->print = true;
>                 cifs_dbg(FYI, "connection to printer\n");
>                 break;
>         default:
> @@ -1913,16 +1899,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>         if (rc)
>                 return rc;
>
> -       if (use_ipc) {
> -               if (ses->ipc_tid == 0) {
> -                       cifs_small_buf_release(req);
> -                       return -ENOTCONN;
> -               }
> -
> -               cifs_dbg(FYI, "replacing tid 0x%x with IPC tid 0x%x\n",
> -                        req->hdr.sync_hdr.TreeId, ses->ipc_tid);
> -               req->hdr.sync_hdr.TreeId = ses->ipc_tid;
> -       }
>         if (encryption_required(tcon))
>                 flags |= CIFS_TRANSFORM_REQ;
>
> --
> 2.12.3
>
> --
> 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 v1 2/3] CIFS: make IPC a regular tcon

[Aurélien Aptel]

Pavel Shilovskiy <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org> writes:
> Minor: it should probably work this way, but since we set ses->tcon_ipc under a spinlock in cifs_setup_ipc(), it would be better to do something similar here.

I was actually thinking that the spinlock might not be necessary for
that pointer. It's always only set at session creation (new tcon) and
destruction (to null). In between, it's only going to be read. What do
you think?

>> -       /*
>> -        * BB - map targetUNCs to dfs_info3 structures, here or in
>> -        * ses->server->ops->get_dfs_refer.
>> -        */
>
> Is this no longer needed?

If you are talking about the tcon creation, it is now done all the time
at session creation. If you are talking about that specific comment, I
believe it's already done now in parse_dfs_referrals(). That function
parses the packet into dfs_info3_param structures.

-- 
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 v1 2/3] CIFS: make IPC a regular tcon

[Pavel Shilovsky]

2018-01-18 2:30 GMT-08:00 Aurélien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Pavel Shilovskiy <pshilov-0li6OtcxBFHby3iVrkZq2A@public.gmane.org> writes:
>> Minor: it should probably work this way, but since we set ses->tcon_ipc under a spinlock in cifs_setup_ipc(), it would be better to do something similar here.
>
> I was actually thinking that the spinlock might not be necessary for
> that pointer. It's always only set at session creation (new tcon) and
> destruction (to null). In between, it's only going to be read. What do
> you think?

I agree that accessing ses->tcon_ipc is not necessary to be under
spinlock. Btw, why should we put this tcon to the list at the 1st
place? We can leave to be accessed only by ses->tcon_ipc and do not
bother with spinlocks at all.

>
>>> -       /*
>>> -        * BB - map targetUNCs to dfs_info3 structures, here or in
>>> -        * ses->server->ops->get_dfs_refer.
>>> -        */
>>
>> Is this no longer needed?
>
> If you are talking about the tcon creation, it is now done all the time
> at session creation. If you are talking about that specific comment, I
> believe it's already done now in parse_dfs_referrals(). That function
> parses the packet into dfs_info3_param structures.
>

I was talking about the comment.


--
Best regards,
Pavel Shilovsky

Re: [PATCH v1 2/3] CIFS: make IPC a regular tcon

[Aurélien Aptel]

Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> I agree that accessing ses->tcon_ipc is not necessary to be under
> spinlock. Btw, why should we put this tcon to the list at the 1st
> place? We can leave to be accessed only by ses->tcon_ipc and do not
> bother with spinlocks at all.

I thought about it. Initially I thought I could reuse tcon get/put
functions but since its not the case (no mount points refer to the IPC
tcon so it's special) I agree it would make more sense now.

-- 
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)

[PATCH v1 3/3] CIFS: use tcon_ipc instead of use_ipc parameter from SMB2_ioctl

[Aurelien Aptel]

Since IPC now has a tcon object, the caller can just pass it.

Link: https://bugzilla.samba.org/show_bug.cgi?id=12917
Fixes: 9d49640a21bf ("CIFS: implement get_dfs_refer for SMB2+")
Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2file.c  |  2 +-
 fs/cifs/smb2ops.c   | 50 ++++++++++++++++++++------------------------------
 fs/cifs/smb2pdu.c   |  4 +---
 fs/cifs/smb2proto.h |  3 +--
 4 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index b4b1f0305f29..12af5dba742b 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -74,7 +74,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
 		nr_ioctl_req.Reserved = 0;
 		rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
 			fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
 			NULL, NULL /* no return info */);
 		if (rc == -EOPNOTSUPP) {
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ed88ab8a4774..7e51bb7a587c 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -283,7 +283,6 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 			FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL /* no data input */, 0 /* no data input */,
 			(char **)&out_buf, &ret_data_len);
 	if (rc != 0)
@@ -782,7 +781,6 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL, 0 /* no input */,
 			(char **)&res_key, &ret_data_len);
 
@@ -848,8 +846,7 @@ smb2_copychunk_range(const unsigned int xid,
 		/* Request server copy to target from src identified by key */
 		rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
-			true /* is_fsctl */, false /* use_ipc */,
-			(char *)pcchunk,
+			true /* is_fsctl */, (char *)pcchunk,
 			sizeof(struct copychunk_ioctl),	(char **)&retbuf,
 			&ret_data_len);
 		if (rc == 0) {
@@ -1006,7 +1003,7 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
-			true /* is_fctl */, false /* use_ipc */,
+			true /* is_fctl */,
 			&setsparse, 1, NULL, NULL);
 	if (rc) {
 		tcon->broken_sparse_sup = true;
@@ -1077,7 +1074,7 @@ smb2_duplicate_extents(const unsigned int xid,
 	rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid,
 			FSCTL_DUPLICATE_EXTENTS_TO_FILE,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&dup_ext_buf,
 			sizeof(struct duplicate_extents_to_file),
 			NULL,
@@ -1112,7 +1109,7 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
 	return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SET_INTEGRITY_INFORMATION,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&integr_info,
 			sizeof(struct fsctl_set_integrity_information_req),
 			NULL,
@@ -1132,7 +1129,7 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SRV_ENUMERATE_SNAPSHOTS,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			NULL, 0 /* no input data */,
 			(char **)&retbuf,
 			&ret_data_len);
@@ -1351,16 +1348,20 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
 
 	/*
-	 * Use any tcon from the current session. Here, the first one.
+	 * Try to use the IPC tcon, otherwise just use any
 	 */
-	spin_lock(&cifs_tcp_ses_lock);
-	tcon = list_first_entry_or_null(&ses->tcon_list, struct cifs_tcon,
-					tcon_list);
-	if (tcon)
-		tcon->tc_count++;
-	spin_unlock(&cifs_tcp_ses_lock);
+	tcon = ses->tcon_ipc;
+	if (tcon == NULL) {
+		spin_lock(&cifs_tcp_ses_lock);
+		tcon = list_first_entry_or_null(&ses->tcon_list,
+						struct cifs_tcon,
+						tcon_list);
+		if (tcon)
+			tcon->tc_count++;
+		spin_unlock(&cifs_tcp_ses_lock);
+	}
 
-	if (!tcon) {
+	if (tcon == NULL) {
 		cifs_dbg(VFS, "session %p has no tcon available for a dfs referral request\n",
 			 ses);
 		rc = -ENOTCONN;
@@ -1389,20 +1390,11 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
 
 	do {
-		/* try first with IPC */
 		rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 				FSCTL_DFS_GET_REFERRALS,
-				true /* is_fsctl */, true /* use_ipc */,
+				true /* is_fsctl */,
 				(char *)dfs_req, dfs_req_size,
 				(char **)&dfs_rsp, &dfs_rsp_size);
-		if (rc == -ENOTCONN) {
-			/* try with normal tcon */
-			rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
-					FSCTL_DFS_GET_REFERRALS,
-					true /* is_fsctl */, false /*use_ipc*/,
-					(char *)dfs_req, dfs_req_size,
-					(char **)&dfs_rsp, &dfs_rsp_size);
-		}
 	} while (rc == -EAGAIN);
 
 	if (rc) {
@@ -1713,8 +1705,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
@@ -1748,8 +1739,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index a5c94f8308eb..a8a632c44c1b 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -713,7 +713,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 		FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
-		false /* use_ipc */,
 		(char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req),
 		(char **)&pneg_rsp, &rsplen);
 
@@ -1863,7 +1862,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
  */
 int
 SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
-	   u64 volatile_fid, u32 opcode, bool is_fsctl, bool use_ipc,
+	   u64 volatile_fid, u32 opcode, bool is_fsctl,
 	   char *in_data, u32 indatalen,
 	   char **out_data, u32 *plen /* returned data len */)
 {
@@ -2028,7 +2027,6 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SET_COMPRESSION, true /* is_fsctl */,
-			false /* use_ipc */,
 			(char *)&fsctl_input /* data input */,
 			2 /* in data len */, &ret_data /* out data */, NULL);
 
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index e9ab5227e7a8..05287b01f596 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -125,8 +125,7 @@ extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms,
 		     struct smb2_err_rsp **err_buf);
 extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid, u32 opcode,
-		     bool is_fsctl, bool use_ipc,
-		     char *in_data, u32 indatalen,
+		     bool is_fsctl, char *in_data, u32 indatalen,
 		     char **out_data, u32 *plen /* returned data len */);
 extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 		      u64 persistent_file_id, u64 volatile_file_id);
-- 
2.12.3

Re: [PATCH v1 0/3] Make IPC a regular tcon and fix SMB2 domain-based DFS

[Ronnie Sahlberg]

Very Nice. I like that it also gets rid of a lot of magic tcon==NULL.

Reviewed-by: Ronnie Sahlberg <lsahlber-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>



----- Original Message -----
From: "Aurelien Aptel" <aaptel-IBi9RG/b67k@public.gmane.org>
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, "Aurelien Aptel" <aaptel-IBi9RG/b67k@public.gmane.org>
Sent: Thursday, 18 January, 2018 4:21:57 AM
Subject: [PATCH v1 0/3] Make IPC a regular tcon and fix SMB2 domain-based DFS

Hi,

The DFS code for smb2 doesn't work [1] when cifs.ko connects to a
domain-based DFS where you have an extra level on indirection (so 3
machines: AD, nameserver and the final storage).

* In SMB1 code, the DFS request is made without the tcon object. It works
  directly on the session.
* In SMB2 code, we use SMB2_ioctl() which works on a tcon.

For domain-based DFS UNC \\DOMAIN\namespace\link, cifs.ko first tries to
connect to \\DOMAIN\namespace. DOMAIN doesn't have a "namespace" share
so no valid tcon object is created.

* SMB1 doesn't care as it's working on the session object and just use
  the ses->ipc_tid number to make the DFS request.
* SMB2 code on the other hand fails as SMB2_ioctl() needs a tcon
  object (even if you pass use_ipc=true) because it uses the tcon you
  pass and simply overwrites the packet tid with ipc_tid. There is no
  valid tcon object at that time since DOMAIN rejected the "namespace"
  tcon request.

So to solve this I thought I would get rid of ses->ipc_tid and make a
real IPC tcon, that would be added to the session tcon_list and be
managed like any other tcon (almost).

This patch series cleans some definitions and makes the IPC connection
a regular tcon owned by the session. It removes the use_ipc param from
SMB2_ioct() (you can pass ses->tcon_ipc as the tcon param).

I've made the choice of having the IPC tcon completely associated to
the session instead of any other tcon. Its refcount is not really used
as it's always created as soon as we have a valid session and always
destroy when the session is destroyed and no mount points directly
refer to it.

All patches compile when applied sequentially, no warnings with
checkpatch.pl, no warnings with make C=1.

I've tested this against Windows Server 2016 vms with smb1, 2, 3 (with
and without encryption) and it seems to be working properly.

I've also quickly tested the reconnection code path but it probably
needs some work still (making sure DFS works after a reconnection) as
there are multiple ways to trigger the reconnection (I've been using
qemu monitor "set_link xyz off" which is analogous to unplugging the
network cable IIUC).

Please take a look, review, test :)

1: https://bugzilla.samba.org/show_bug.cgi?id=12917

Aurelien Aptel (3):
  CIFS: set SERVER_NAME_LENGTH to serverName actual size
  CIFS: make IPC a regular tcon
  CIFS: use tcon_ipc instead of use_ipc parameter from SMB2_ioctl

 fs/cifs/cifsglob.h  |  14 ++---
 fs/cifs/cifssmb.c   |   5 +-
 fs/cifs/connect.c   | 159 ++++++++++++++++++++++++++++++++++++++++------------
 fs/cifs/inode.c     |   2 +-
 fs/cifs/smb2file.c  |   2 +-
 fs/cifs/smb2ops.c   |  50 +++++++----------
 fs/cifs/smb2pdu.c   |  36 ++----------
 fs/cifs/smb2proto.h |   3 +-
 8 files changed, 160 insertions(+), 111 deletions(-)

-- 
2.12.3

--
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

[PATCH v2 0/2] Make IPC a regular tcon and fix SMB2 domain-based DFS

[Aurelien Aptel]

changes since v1:

* squashed definition change of SERVER_NAME_LENGTH in first patch
* ipc tcon no longer added the the session tcon list
* ipc tcon no longer protected by spinlock
* decrement tcon refcount only if it was incremented earlier (ie not
  an ipc tcon) in smb2_get_dfs_refer()

Aurelien Aptel (2):
  CIFS: make IPC a regular tcon
  CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

 fs/cifs/cifsglob.h  |  14 ++---
 fs/cifs/cifssmb.c   |   5 +-
 fs/cifs/connect.c   | 148 +++++++++++++++++++++++++++++++++++++++-------------
 fs/cifs/inode.c     |   2 +-
 fs/cifs/smb2file.c  |   2 +-
 fs/cifs/smb2ops.c   |  53 ++++++++-----------
 fs/cifs/smb2pdu.c   |  36 ++-----------
 fs/cifs/smb2proto.h |   3 +-
 8 files changed, 151 insertions(+), 112 deletions(-)

-- 
2.12.3

[PATCH v2 1/2] CIFS: make IPC a regular tcon

[Aurelien Aptel]

* Remove ses->ipc_tid.
* Make IPC$ regular tcon.
* Add a direct pointer to it in ses->tcon_ipc.
* Distinguish PIPE tcon from IPC tcon by adding a tcon->pipe flag. All
  IPC tcons are pipes but not all pipes are IPC.
* All TreeConnect functions now cannot take a NULL tcon object.

The IPC tcon has the same lifetime as the session it belongs to. It is
created when the session is created and destroyed when the session is
destroyed.

Since no mounts directly refer to the IPC tcon, its refcount should
always be 0. Thus we make sure cifs_put_tcon() skips it.

If the mount request resulting in a new session being created requires
encryption, try to require it too for IPC.

* set SERVER_NAME_LENGTH to serverName actual size

The maximum length of an ipv6 string representation is defined in
INET6_ADDRSTRLEN as 45+1 for null but lets keep what we know works.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifsglob.h |  14 ++---
 fs/cifs/cifssmb.c  |   5 +-
 fs/cifs/connect.c  | 148 +++++++++++++++++++++++++++++++++++++++--------------
 fs/cifs/inode.c    |   2 +-
 fs/cifs/smb2pdu.c  |  32 ++----------
 5 files changed, 126 insertions(+), 75 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index b16583594d1a..804ec79af5e1 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -64,8 +64,8 @@
 #define RFC1001_NAME_LEN 15
 #define RFC1001_NAME_LEN_WITH_NULL (RFC1001_NAME_LEN + 1)
 
-/* currently length of NIP6_FMT */
-#define SERVER_NAME_LENGTH 40
+/* maximum length of ip addr as a string (including ipv6 and sctp) */
+#define SERVER_NAME_LENGTH 80
 #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
 
 /* echo interval in seconds */
@@ -822,12 +822,12 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
 struct cifs_ses {
 	struct list_head smb_ses_list;
 	struct list_head tcon_list;
+	struct cifs_tcon *tcon_ipc;
 	struct mutex session_mutex;
 	struct TCP_Server_Info *server;	/* pointer to server info */
 	int ses_count;		/* reference counter */
 	enum statusEnum status;
 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
-	__u32 ipc_tid;		/* special tid for connection to IPC share */
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
 	char *serverDomain;	/* security realm of server */
@@ -835,8 +835,7 @@ struct cifs_ses {
 	kuid_t linux_uid;	/* overriding owner of files on the mount */
 	kuid_t cred_uid;	/* owner of credentials */
 	unsigned int capabilities;
-	char serverName[SERVER_NAME_LEN_WITH_NULL * 2];	/* BB make bigger for
-				TCP names - will ipv6 and sctp addresses fit? */
+	char serverName[SERVER_NAME_LEN_WITH_NULL];
 	char *user_name;	/* must not be null except during init of sess
 				   and after mount option parsing we fill it */
 	char *domainName;
@@ -931,7 +930,9 @@ struct cifs_tcon {
 	FILE_SYSTEM_DEVICE_INFO fsDevInfo;
 	FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
 	FILE_SYSTEM_UNIX_INFO fsUnixInfo;
-	bool ipc:1;		/* set if connection to IPC$ eg for RPC/PIPES */
+	bool ipc:1;   /* set if connection to IPC$ share (always also pipe) */
+	bool pipe:1;  /* set if connection to pipe share */
+	bool print:1; /* set if connection to printer share */
 	bool retry:1;
 	bool nocase:1;
 	bool seal:1;      /* transport encryption for this mounted share */
@@ -944,7 +945,6 @@ struct cifs_tcon {
 	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 */
-	bool print:1;		/* set if connection to printer share */
 	__le32 capabilities;
 	__u32 share_flags;
 	__u32 maximal_access;
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 35dc5bf01ee2..2f8dea905ef9 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4822,8 +4822,9 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	*target_nodes = NULL;
 
 	cifs_dbg(FYI, "In GetDFSRefer the path %s\n", search_name);
-	if (ses == NULL)
+	if (ses == NULL || ses->tcon_ipc == NULL)
 		return -ENODEV;
+
 getDFSRetry:
 	rc = smb_init(SMB_COM_TRANSACTION2, 15, NULL, (void **) &pSMB,
 		      (void **) &pSMBr);
@@ -4833,7 +4834,7 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	/* server pointer checked in called function,
 	but should never be null here anyway */
 	pSMB->hdr.Mid = get_next_mid(ses->server);
-	pSMB->hdr.Tid = ses->ipc_tid;
+	pSMB->hdr.Tid = ses->tcon_ipc->tid;
 	pSMB->hdr.Uid = ses->Suid;
 	if (ses->capabilities & CAP_STATUS32)
 		pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 0bfc2280436d..945fe2c6efcd 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -353,7 +353,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	list_for_each(tmp, &server->smb_ses_list) {
 		ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
 		ses->need_reconnect = true;
-		ses->ipc_tid = 0;
 		list_for_each(tmp2, &ses->tcon_list) {
 			tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
 			tcon->need_reconnect = true;
@@ -2381,6 +2380,93 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
 	return 1;
 }
 
+/**
+ * cifs_setup_ipc - helper to setup the IPC tcon for the session
+ *
+ * A new IPC connection is made and stored in the session
+ * tcon_ipc. The IPC tcon has the same lifetime as the session.
+ */
+static int
+cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon;
+	struct nls_table *nls_codepage;
+	char unc[SERVER_NAME_LENGTH + sizeof("//x/IPC$")] = {0};
+	bool seal = false;
+
+	/*
+	 * If the mount request that resulted in the creation of the
+	 * session requires encryption, force IPC to be encrypted too.
+	 */
+	if (volume_info->seal) {
+		if (ses->server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)
+			seal = true;
+		else {
+			cifs_dbg(VFS,
+				 "IPC: server doesn't support encryption\n");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	tcon = tconInfoAlloc();
+	if (tcon == NULL)
+		return -ENOMEM;
+
+	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
+
+	/* cannot fail */
+	nls_codepage = load_nls_default();
+
+	xid = get_xid();
+	tcon->ses = ses;
+	tcon->ipc = true;
+	tcon->seal = seal;
+	rc = ses->server->ops->tree_connect(xid, ses, unc, tcon, nls_codepage);
+	free_xid(xid);
+
+	if (rc) {
+		cifs_dbg(VFS, "failed to connect to IPC (rc=%d)\n", rc);
+		tconInfoFree(tcon);
+		goto out;
+	}
+
+	cifs_dbg(FYI, "IPC tcon rc = %d ipc tid = %d\n", rc, tcon->tid);
+
+	ses->tcon_ipc = tcon;
+out:
+	unload_nls(nls_codepage);
+	return rc;
+}
+
+/**
+ * cifs_free_ipc - helper to release the session IPC tcon
+ *
+ * Needs to be called everytime a session is destroyed
+ */
+static int
+cifs_free_ipc(struct cifs_ses *ses)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon = ses->tcon_ipc;
+
+	if (tcon == NULL)
+		return 0;
+
+	if (ses->server->ops->tree_disconnect) {
+		xid = get_xid();
+		rc = ses->server->ops->tree_disconnect(xid, tcon);
+		free_xid(xid);
+	}
+
+	if (rc)
+		cifs_dbg(FYI, "failed to disconnect IPC tcon (rc=%d)\n", rc);
+
+	tconInfoFree(tcon);
+	ses->tcon_ipc = NULL;
+	return rc;
+}
+
 static struct cifs_ses *
 cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
 {
@@ -2421,6 +2507,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
 		ses->status = CifsExiting;
 	spin_unlock(&cifs_tcp_ses_lock);
 
+	cifs_free_ipc(ses);
+
 	if (ses->status == CifsExiting && server->ops->logoff) {
 		xid = get_xid();
 		rc = server->ops->logoff(xid, ses);
@@ -2665,6 +2753,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	free_xid(xid);
+
+	cifs_setup_ipc(ses, volume_info);
+
 	return ses;
 
 get_ses_fail:
@@ -2709,8 +2800,16 @@ void
 cifs_put_tcon(struct cifs_tcon *tcon)
 {
 	unsigned int xid;
-	struct cifs_ses *ses = tcon->ses;
+	struct cifs_ses *ses;
+
+	/*
+	 * IPC tcon share the lifetime of their session and are
+	 * destroyed in the session put function
+	 */
+	if (tcon == NULL || tcon->ipc)
+		return;
 
+	ses = tcon->ses;
 	cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
 	spin_lock(&cifs_tcp_ses_lock);
 	if (--tcon->tc_count > 0) {
@@ -2986,39 +3085,17 @@ get_dfs_path(const unsigned int xid, struct cifs_ses *ses, const char *old_path,
 	     const struct nls_table *nls_codepage, unsigned int *num_referrals,
 	     struct dfs_info3_param **referrals, int remap)
 {
-	char *temp_unc;
 	int rc = 0;
 
-	if (!ses->server->ops->tree_connect || !ses->server->ops->get_dfs_refer)
+	if (!ses->server->ops->get_dfs_refer)
 		return -ENOSYS;
 
 	*num_referrals = 0;
 	*referrals = NULL;
 
-	if (ses->ipc_tid == 0) {
-		temp_unc = kmalloc(2 /* for slashes */ +
-			strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
-				+ 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
-		if (temp_unc == NULL)
-			return -ENOMEM;
-		temp_unc[0] = '\\';
-		temp_unc[1] = '\\';
-		strcpy(temp_unc + 2, ses->serverName);
-		strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
-		rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
-						    nls_codepage);
-		cifs_dbg(FYI, "Tcon rc = %d ipc_tid = %d\n", rc, ses->ipc_tid);
-		kfree(temp_unc);
-	}
-	if (rc == 0)
-		rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
-						     referrals, num_referrals,
-						     nls_codepage, remap);
-	/*
-	 * BB - map targetUNCs to dfs_info3 structures, here or in
-	 * ses->server->ops->get_dfs_refer.
-	 */
-
+	rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
+					     referrals, num_referrals,
+					     nls_codepage, remap);
 	return rc;
 }
 
@@ -3783,7 +3860,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 		tcon->unix_ext = 0; /* server does not support them */
 
 	/* do not care if a following call succeed - informational */
-	if (!tcon->ipc && server->ops->qfs_tcon)
+	if (!tcon->pipe && server->ops->qfs_tcon)
 		server->ops->qfs_tcon(xid, tcon);
 
 	cifs_sb->wsize = server->ops->negotiate_wsize(tcon, volume_info);
@@ -3913,8 +3990,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 }
 
 /*
- * Issue a TREE_CONNECT request. Note that for IPC$ shares, that the tcon
- * pointer may be NULL.
+ * Issue a TREE_CONNECT request.
  */
 int
 CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
@@ -3950,7 +4026,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 	pSMB->AndXCommand = 0xFF;
 	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
 	bcc_ptr = &pSMB->Password[0];
-	if (!tcon || (ses->server->sec_mode & SECMODE_USER)) {
+	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
 		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
 		*bcc_ptr = 0; /* password is null byte */
 		bcc_ptr++;              /* skip password */
@@ -4022,7 +4098,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			 0);
 
 	/* above now done in SendReceive */
-	if ((rc == 0) && (tcon != NULL)) {
+	if (rc == 0) {
 		bool is_unicode;
 
 		tcon->tidStatus = CifsGood;
@@ -4042,7 +4118,8 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
 			    (bcc_ptr[2] == 'C')) {
 				cifs_dbg(FYI, "IPC connection\n");
-				tcon->ipc = 1;
+				tcon->ipc = true;
+				tcon->pipe = true;
 			}
 		} else if (length == 2) {
 			if ((bcc_ptr[0] == 'A') && (bcc_ptr[1] == ':')) {
@@ -4069,9 +4146,6 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		else
 			tcon->Flags = 0;
 		cifs_dbg(FYI, "Tcon flags: 0x%x\n", tcon->Flags);
-	} else if ((rc == 0) && tcon == NULL) {
-		/* all we need to save for IPC$ connection */
-		ses->ipc_tid = smb_buffer_response->Tid;
 	}
 
 	cifs_buf_release(smb_buffer);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ecb99079363a..8f9a8cc7cc62 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1049,7 +1049,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 	tcon->resource_id = CIFS_I(inode)->uniqueid;
 #endif
 
-	if (rc && tcon->ipc) {
+	if (rc && tcon->pipe) {
 		cifs_dbg(FYI, "ipc connection - fake read inode\n");
 		spin_lock(&inode->i_lock);
 		inode->i_mode |= S_IFDIR;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 01346b8b6edb..a5c94f8308eb 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1283,8 +1283,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	}
 
 	/* SMB2 TREE_CONNECT request must be called with TreeId == 0 */
-	if (tcon)
-		tcon->tid = 0;
+	tcon->tid = 0;
 
 	rc = small_smb2_init(SMB2_TREE_CONNECT, tcon, (void **) &req);
 	if (rc) {
@@ -1292,15 +1291,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		return rc;
 	}
 
-	if (tcon == NULL) {
-		if ((ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
-			flags |= CIFS_TRANSFORM_REQ;
-
-		/* since no tcon, smb2_init can not do this, so do here */
-		req->hdr.sync_hdr.SessionId = ses->Suid;
-		if (ses->server->sign)
-			req->hdr.sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
-	} else if (encryption_required(tcon))
+	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
 	iov[0].iov_base = (char *)req;
@@ -1328,21 +1319,16 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		goto tcon_error_exit;
 	}
 
-	if (tcon == NULL) {
-		ses->ipc_tid = rsp->hdr.sync_hdr.TreeId;
-		goto tcon_exit;
-	}
-
 	switch (rsp->ShareType) {
 	case SMB2_SHARE_TYPE_DISK:
 		cifs_dbg(FYI, "connection to disk share\n");
 		break;
 	case SMB2_SHARE_TYPE_PIPE:
-		tcon->ipc = true;
+		tcon->pipe = true;
 		cifs_dbg(FYI, "connection to pipe share\n");
 		break;
 	case SMB2_SHARE_TYPE_PRINT:
-		tcon->ipc = true;
+		tcon->print = true;
 		cifs_dbg(FYI, "connection to printer\n");
 		break;
 	default:
@@ -1913,16 +1899,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	if (rc)
 		return rc;
 
-	if (use_ipc) {
-		if (ses->ipc_tid == 0) {
-			cifs_small_buf_release(req);
-			return -ENOTCONN;
-		}
-
-		cifs_dbg(FYI, "replacing tid 0x%x with IPC tid 0x%x\n",
-			 req->hdr.sync_hdr.TreeId, ses->ipc_tid);
-		req->hdr.sync_hdr.TreeId = ses->ipc_tid;
-	}
 	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
-- 
2.12.3

[PATCH v2 2/2] CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

[Aurelien Aptel]

Since IPC now has a tcon object, the caller can just pass it. This
allows domain-based DFS requests to work with smb2+.

Link: https://bugzilla.samba.org/show_bug.cgi?id=12917
Fixes: 9d49640a21bf ("CIFS: implement get_dfs_refer for SMB2+")
Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2file.c  |  2 +-
 fs/cifs/smb2ops.c   | 53 ++++++++++++++++++++++-------------------------------
 fs/cifs/smb2pdu.c   |  4 +---
 fs/cifs/smb2proto.h |  3 +--
 4 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index b4b1f0305f29..12af5dba742b 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -74,7 +74,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
 		nr_ioctl_req.Reserved = 0;
 		rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
 			fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
 			NULL, NULL /* no return info */);
 		if (rc == -EOPNOTSUPP) {
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ed88ab8a4774..1d9907da96f4 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -283,7 +283,6 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 			FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL /* no data input */, 0 /* no data input */,
 			(char **)&out_buf, &ret_data_len);
 	if (rc != 0)
@@ -782,7 +781,6 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL, 0 /* no input */,
 			(char **)&res_key, &ret_data_len);
 
@@ -848,8 +846,7 @@ smb2_copychunk_range(const unsigned int xid,
 		/* Request server copy to target from src identified by key */
 		rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
-			true /* is_fsctl */, false /* use_ipc */,
-			(char *)pcchunk,
+			true /* is_fsctl */, (char *)pcchunk,
 			sizeof(struct copychunk_ioctl),	(char **)&retbuf,
 			&ret_data_len);
 		if (rc == 0) {
@@ -1006,7 +1003,7 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
-			true /* is_fctl */, false /* use_ipc */,
+			true /* is_fctl */,
 			&setsparse, 1, NULL, NULL);
 	if (rc) {
 		tcon->broken_sparse_sup = true;
@@ -1077,7 +1074,7 @@ smb2_duplicate_extents(const unsigned int xid,
 	rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid,
 			FSCTL_DUPLICATE_EXTENTS_TO_FILE,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&dup_ext_buf,
 			sizeof(struct duplicate_extents_to_file),
 			NULL,
@@ -1112,7 +1109,7 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
 	return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SET_INTEGRITY_INFORMATION,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&integr_info,
 			sizeof(struct fsctl_set_integrity_information_req),
 			NULL,
@@ -1132,7 +1129,7 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SRV_ENUMERATE_SNAPSHOTS,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			NULL, 0 /* no input data */,
 			(char **)&retbuf,
 			&ret_data_len);
@@ -1351,16 +1348,20 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
 
 	/*
-	 * Use any tcon from the current session. Here, the first one.
+	 * Try to use the IPC tcon, otherwise just use any
 	 */
-	spin_lock(&cifs_tcp_ses_lock);
-	tcon = list_first_entry_or_null(&ses->tcon_list, struct cifs_tcon,
-					tcon_list);
-	if (tcon)
-		tcon->tc_count++;
-	spin_unlock(&cifs_tcp_ses_lock);
+	tcon = ses->tcon_ipc;
+	if (tcon == NULL) {
+		spin_lock(&cifs_tcp_ses_lock);
+		tcon = list_first_entry_or_null(&ses->tcon_list,
+						struct cifs_tcon,
+						tcon_list);
+		if (tcon)
+			tcon->tc_count++;
+		spin_unlock(&cifs_tcp_ses_lock);
+	}
 
-	if (!tcon) {
+	if (tcon == NULL) {
 		cifs_dbg(VFS, "session %p has no tcon available for a dfs referral request\n",
 			 ses);
 		rc = -ENOTCONN;
@@ -1389,20 +1390,11 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
 
 	do {
-		/* try first with IPC */
 		rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 				FSCTL_DFS_GET_REFERRALS,
-				true /* is_fsctl */, true /* use_ipc */,
+				true /* is_fsctl */,
 				(char *)dfs_req, dfs_req_size,
 				(char **)&dfs_rsp, &dfs_rsp_size);
-		if (rc == -ENOTCONN) {
-			/* try with normal tcon */
-			rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
-					FSCTL_DFS_GET_REFERRALS,
-					true /* is_fsctl */, false /*use_ipc*/,
-					(char *)dfs_req, dfs_req_size,
-					(char **)&dfs_rsp, &dfs_rsp_size);
-		}
 	} while (rc == -EAGAIN);
 
 	if (rc) {
@@ -1421,7 +1413,8 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	}
 
  out:
-	if (tcon) {
+	if (tcon && !tcon->ipc) {
+		/* ipc tcons are not refcounted */
 		spin_lock(&cifs_tcp_ses_lock);
 		tcon->tc_count--;
 		spin_unlock(&cifs_tcp_ses_lock);
@@ -1713,8 +1706,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
@@ -1748,8 +1740,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index a5c94f8308eb..a8a632c44c1b 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -713,7 +713,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 		FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
-		false /* use_ipc */,
 		(char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req),
 		(char **)&pneg_rsp, &rsplen);
 
@@ -1863,7 +1862,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
  */
 int
 SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
-	   u64 volatile_fid, u32 opcode, bool is_fsctl, bool use_ipc,
+	   u64 volatile_fid, u32 opcode, bool is_fsctl,
 	   char *in_data, u32 indatalen,
 	   char **out_data, u32 *plen /* returned data len */)
 {
@@ -2028,7 +2027,6 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SET_COMPRESSION, true /* is_fsctl */,
-			false /* use_ipc */,
 			(char *)&fsctl_input /* data input */,
 			2 /* in data len */, &ret_data /* out data */, NULL);
 
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index e9ab5227e7a8..05287b01f596 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -125,8 +125,7 @@ extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms,
 		     struct smb2_err_rsp **err_buf);
 extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid, u32 opcode,
-		     bool is_fsctl, bool use_ipc,
-		     char *in_data, u32 indatalen,
+		     bool is_fsctl, char *in_data, u32 indatalen,
 		     char **out_data, u32 *plen /* returned data len */);
 extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 		      u64 persistent_file_id, u64 volatile_file_id);
-- 
2.12.3

Re: [PATCH v2 2/2] CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

[Pavel Shilovsky]

2018-01-19 9:12 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Since IPC now has a tcon object, the caller can just pass it. This
> allows domain-based DFS requests to work with smb2+.
>
> Link: https://bugzilla.samba.org/show_bug.cgi?id=12917
> Fixes: 9d49640a21bf ("CIFS: implement get_dfs_refer for SMB2+")
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/smb2file.c  |  2 +-
>  fs/cifs/smb2ops.c   | 53 ++++++++++++++++++++++-------------------------------
>  fs/cifs/smb2pdu.c   |  4 +---
>  fs/cifs/smb2proto.h |  3 +--
>  4 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index b4b1f0305f29..12af5dba742b 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -74,7 +74,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
>                 nr_ioctl_req.Reserved = 0;
>                 rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
>                         fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
>                         NULL, NULL /* no return info */);
>                 if (rc == -EOPNOTSUPP) {
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index ed88ab8a4774..1d9907da96f4 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -283,7 +283,6 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
>
>         rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>                         FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> -                       false /* use_ipc */,
>                         NULL /* no data input */, 0 /* no data input */,
>                         (char **)&out_buf, &ret_data_len);
>         if (rc != 0)
> @@ -782,7 +781,6 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>                         FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> -                       false /* use_ipc */,
>                         NULL, 0 /* no input */,
>                         (char **)&res_key, &ret_data_len);
>
> @@ -848,8 +846,7 @@ smb2_copychunk_range(const unsigned int xid,
>                 /* Request server copy to target from src identified by key */
>                 rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>                         trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> -                       true /* is_fsctl */, false /* use_ipc */,
> -                       (char *)pcchunk,
> +                       true /* is_fsctl */, (char *)pcchunk,
>                         sizeof(struct copychunk_ioctl), (char **)&retbuf,
>                         &ret_data_len);
>                 if (rc == 0) {
> @@ -1006,7 +1003,7 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> -                       true /* is_fctl */, false /* use_ipc */,
> +                       true /* is_fctl */,
>                         &setsparse, 1, NULL, NULL);
>         if (rc) {
>                 tcon->broken_sparse_sup = true;
> @@ -1077,7 +1074,7 @@ smb2_duplicate_extents(const unsigned int xid,
>         rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>                         trgtfile->fid.volatile_fid,
>                         FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&dup_ext_buf,
>                         sizeof(struct duplicate_extents_to_file),
>                         NULL,
> @@ -1112,7 +1109,7 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
>         return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid,
>                         FSCTL_SET_INTEGRITY_INFORMATION,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&integr_info,
>                         sizeof(struct fsctl_set_integrity_information_req),
>                         NULL,
> @@ -1132,7 +1129,7 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid,
>                         FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         NULL, 0 /* no input data */,
>                         (char **)&retbuf,
>                         &ret_data_len);
> @@ -1351,16 +1348,20 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>         cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
>
>         /*
> -        * Use any tcon from the current session. Here, the first one.
> +        * Try to use the IPC tcon, otherwise just use any
>          */
> -       spin_lock(&cifs_tcp_ses_lock);
> -       tcon = list_first_entry_or_null(&ses->tcon_list, struct cifs_tcon,
> -                                       tcon_list);
> -       if (tcon)
> -               tcon->tc_count++;
> -       spin_unlock(&cifs_tcp_ses_lock);
> +       tcon = ses->tcon_ipc;
> +       if (tcon == NULL) {
> +               spin_lock(&cifs_tcp_ses_lock);
> +               tcon = list_first_entry_or_null(&ses->tcon_list,
> +                                               struct cifs_tcon,
> +                                               tcon_list);
> +               if (tcon)
> +                       tcon->tc_count++;
> +               spin_unlock(&cifs_tcp_ses_lock);
> +       }
>
> -       if (!tcon) {
> +       if (tcon == NULL) {
>                 cifs_dbg(VFS, "session %p has no tcon available for a dfs referral request\n",
>                          ses);
>                 rc = -ENOTCONN;
> @@ -1389,20 +1390,11 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>         memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
>
>         do {
> -               /* try first with IPC */
>                 rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>                                 FSCTL_DFS_GET_REFERRALS,
> -                               true /* is_fsctl */, true /* use_ipc */,
> +                               true /* is_fsctl */,
>                                 (char *)dfs_req, dfs_req_size,
>                                 (char **)&dfs_rsp, &dfs_rsp_size);
> -               if (rc == -ENOTCONN) {
> -                       /* try with normal tcon */
> -                       rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> -                                       FSCTL_DFS_GET_REFERRALS,
> -                                       true /* is_fsctl */, false /*use_ipc*/,
> -                                       (char *)dfs_req, dfs_req_size,
> -                                       (char **)&dfs_rsp, &dfs_rsp_size);
> -               }

Is it safe to try IPC share only and not retry with any normal tcon in
the case of ENOTCONN like the current code does?

Best regards,
Pavel Shilovsky

Re: [PATCH v2 2/2] CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

[Aurélien Aptel]

Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Is it safe to try IPC share only and not retry with any normal tcon in
> the case of ENOTCONN like the current code does?

ENOTCONN was returned when SMB2_ioctl(with_ipc=true) was called and no
ipc was available so I believe it is functionnaly equivalent
currently. Unless other NT_STATUS mapped to ENOTCONN can be returned in
that scenario but I don't think it can.

While rebasing this on top of Steve's for-next I've realized since IPC
is not part of the tcon list anymore it's not automatically
reconnected. I have to do more testing :(

-- 
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)

[PATCH v3 0/3] Make IPC a regular tcon and fix SMB2 domain-based DFS

[Aurelien Aptel]

changes since v2:

* rebased against Steve's for-next
* mark IPC tcon for reconnection like other tcons
* dump IPC tcon in /proc DebugData
* pass IPC tcon instead of NULL to smb_init() in CIFSGetDFSRefer() so
  that it reconnect it if needed

passes make C=1, checkpatch.pl, and reconnection works in these cases
(domain-based DFS with Windows Server 2016):

| test                      | v1.0 | v3.0 |
|---------------------------+------+------|
| mount root, reco, cd link |  ok  |  ok  |
| mount link, reco, ls      |  ok  |  ok  |



changes since v1:

* squashed definition change of SERVER_NAME_LENGTH in first patch
* ipc tcon no longer added the the session tcon list
* ipc tcon no longer protected by spinlock
* decrement tcon refcount only if it was incremented earlier (ie not
  an ipc tcon) in smb2_get_dfs_refer()

Aurelien Aptel (3):
  CIFS: make IPC a regular tcon
  CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl
  CIFS: dump IPC tcon in debug proc file

 fs/cifs/cifs_debug.c |  61 ++++++++++++---------
 fs/cifs/cifsglob.h   |  14 ++---
 fs/cifs/cifssmb.c    |   7 +--
 fs/cifs/connect.c    | 150 ++++++++++++++++++++++++++++++++++++++-------------
 fs/cifs/inode.c      |   2 +-
 fs/cifs/smb2file.c   |   2 +-
 fs/cifs/smb2ops.c    |  53 ++++++++----------
 fs/cifs/smb2pdu.c    |  40 ++++----------
 fs/cifs/smb2proto.h  |   3 +-
 9 files changed, 193 insertions(+), 139 deletions(-)

-- 
2.12.3

[PATCH v3 1/3] CIFS: make IPC a regular tcon

[Aurelien Aptel]

* Remove ses->ipc_tid.
* Make IPC$ regular tcon.
* Add a direct pointer to it in ses->tcon_ipc.
* Distinguish PIPE tcon from IPC tcon by adding a tcon->pipe flag. All
  IPC tcons are pipes but not all pipes are IPC.
* All TreeConnect functions now cannot take a NULL tcon object.

The IPC tcon has the same lifetime as the session it belongs to. It is
created when the session is created and destroyed when the session is
destroyed.

Since no mounts directly refer to the IPC tcon, its refcount should
always be set to initialisation value (1). Thus we make sure
cifs_put_tcon() skips it.

If the mount request resulting in a new session being created requires
encryption, try to require it too for IPC.

* set SERVER_NAME_LENGTH to serverName actual size

The maximum length of an ipv6 string representation is defined in
INET6_ADDRSTRLEN as 45+1 for null but lets keep what we know works.

Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifsglob.h |  14 ++---
 fs/cifs/cifssmb.c  |   7 +--
 fs/cifs/connect.c  | 150 ++++++++++++++++++++++++++++++++++++++++-------------
 fs/cifs/inode.c    |   2 +-
 fs/cifs/smb2pdu.c  |  36 +++----------
 5 files changed, 133 insertions(+), 76 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 678e638c1e69..48f7c197cd2d 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -64,8 +64,8 @@
 #define RFC1001_NAME_LEN 15
 #define RFC1001_NAME_LEN_WITH_NULL (RFC1001_NAME_LEN + 1)
 
-/* currently length of NIP6_FMT */
-#define SERVER_NAME_LENGTH 40
+/* maximum length of ip addr as a string (including ipv6 and sctp) */
+#define SERVER_NAME_LENGTH 80
 #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
 
 /* echo interval in seconds */
@@ -833,12 +833,12 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
 struct cifs_ses {
 	struct list_head smb_ses_list;
 	struct list_head tcon_list;
+	struct cifs_tcon *tcon_ipc;
 	struct mutex session_mutex;
 	struct TCP_Server_Info *server;	/* pointer to server info */
 	int ses_count;		/* reference counter */
 	enum statusEnum status;
 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
-	__u32 ipc_tid;		/* special tid for connection to IPC share */
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
 	char *serverDomain;	/* security realm of server */
@@ -846,8 +846,7 @@ struct cifs_ses {
 	kuid_t linux_uid;	/* overriding owner of files on the mount */
 	kuid_t cred_uid;	/* owner of credentials */
 	unsigned int capabilities;
-	char serverName[SERVER_NAME_LEN_WITH_NULL * 2];	/* BB make bigger for
-				TCP names - will ipv6 and sctp addresses fit? */
+	char serverName[SERVER_NAME_LEN_WITH_NULL];
 	char *user_name;	/* must not be null except during init of sess
 				   and after mount option parsing we fill it */
 	char *domainName;
@@ -942,7 +941,9 @@ struct cifs_tcon {
 	FILE_SYSTEM_DEVICE_INFO fsDevInfo;
 	FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
 	FILE_SYSTEM_UNIX_INFO fsUnixInfo;
-	bool ipc:1;		/* set if connection to IPC$ eg for RPC/PIPES */
+	bool ipc:1;   /* set if connection to IPC$ share (always also pipe) */
+	bool pipe:1;  /* set if connection to pipe share */
+	bool print:1; /* set if connection to printer share */
 	bool retry:1;
 	bool nocase:1;
 	bool seal:1;      /* transport encryption for this mounted share */
@@ -955,7 +956,6 @@ struct cifs_tcon {
 	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 */
-	bool print:1;		/* set if connection to printer share */
 	__le32 capabilities;
 	__u32 share_flags;
 	__u32 maximal_access;
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 49cf999f3d46..4e0922d24eb2 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4833,10 +4833,11 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	*target_nodes = NULL;
 
 	cifs_dbg(FYI, "In GetDFSRefer the path %s\n", search_name);
-	if (ses == NULL)
+	if (ses == NULL || ses->tcon_ipc == NULL)
 		return -ENODEV;
+
 getDFSRetry:
-	rc = smb_init(SMB_COM_TRANSACTION2, 15, NULL, (void **) &pSMB,
+	rc = smb_init(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc, (void **) &pSMB,
 		      (void **) &pSMBr);
 	if (rc)
 		return rc;
@@ -4844,7 +4845,7 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
 	/* server pointer checked in called function,
 	but should never be null here anyway */
 	pSMB->hdr.Mid = get_next_mid(ses->server);
-	pSMB->hdr.Tid = ses->ipc_tid;
+	pSMB->hdr.Tid = ses->tcon_ipc->tid;
 	pSMB->hdr.Uid = ses->Suid;
 	if (ses->capabilities & CAP_STATUS32)
 		pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 63c5d85fe25e..8b5e401f547a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -354,11 +354,12 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	list_for_each(tmp, &server->smb_ses_list) {
 		ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
 		ses->need_reconnect = true;
-		ses->ipc_tid = 0;
 		list_for_each(tmp2, &ses->tcon_list) {
 			tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
 			tcon->need_reconnect = true;
 		}
+		if (ses->tcon_ipc)
+			ses->tcon_ipc->need_reconnect = true;
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
 
@@ -2426,6 +2427,93 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
 	return 1;
 }
 
+/**
+ * cifs_setup_ipc - helper to setup the IPC tcon for the session
+ *
+ * A new IPC connection is made and stored in the session
+ * tcon_ipc. The IPC tcon has the same lifetime as the session.
+ */
+static int
+cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon;
+	struct nls_table *nls_codepage;
+	char unc[SERVER_NAME_LENGTH + sizeof("//x/IPC$")] = {0};
+	bool seal = false;
+
+	/*
+	 * If the mount request that resulted in the creation of the
+	 * session requires encryption, force IPC to be encrypted too.
+	 */
+	if (volume_info->seal) {
+		if (ses->server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)
+			seal = true;
+		else {
+			cifs_dbg(VFS,
+				 "IPC: server doesn't support encryption\n");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	tcon = tconInfoAlloc();
+	if (tcon == NULL)
+		return -ENOMEM;
+
+	snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
+
+	/* cannot fail */
+	nls_codepage = load_nls_default();
+
+	xid = get_xid();
+	tcon->ses = ses;
+	tcon->ipc = true;
+	tcon->seal = seal;
+	rc = ses->server->ops->tree_connect(xid, ses, unc, tcon, nls_codepage);
+	free_xid(xid);
+
+	if (rc) {
+		cifs_dbg(VFS, "failed to connect to IPC (rc=%d)\n", rc);
+		tconInfoFree(tcon);
+		goto out;
+	}
+
+	cifs_dbg(FYI, "IPC tcon rc = %d ipc tid = %d\n", rc, tcon->tid);
+
+	ses->tcon_ipc = tcon;
+out:
+	unload_nls(nls_codepage);
+	return rc;
+}
+
+/**
+ * cifs_free_ipc - helper to release the session IPC tcon
+ *
+ * Needs to be called everytime a session is destroyed
+ */
+static int
+cifs_free_ipc(struct cifs_ses *ses)
+{
+	int rc = 0, xid;
+	struct cifs_tcon *tcon = ses->tcon_ipc;
+
+	if (tcon == NULL)
+		return 0;
+
+	if (ses->server->ops->tree_disconnect) {
+		xid = get_xid();
+		rc = ses->server->ops->tree_disconnect(xid, tcon);
+		free_xid(xid);
+	}
+
+	if (rc)
+		cifs_dbg(FYI, "failed to disconnect IPC tcon (rc=%d)\n", rc);
+
+	tconInfoFree(tcon);
+	ses->tcon_ipc = NULL;
+	return rc;
+}
+
 static struct cifs_ses *
 cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
 {
@@ -2466,6 +2554,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
 		ses->status = CifsExiting;
 	spin_unlock(&cifs_tcp_ses_lock);
 
+	cifs_free_ipc(ses);
+
 	if (ses->status == CifsExiting && server->ops->logoff) {
 		xid = get_xid();
 		rc = server->ops->logoff(xid, ses);
@@ -2710,6 +2800,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	free_xid(xid);
+
+	cifs_setup_ipc(ses, volume_info);
+
 	return ses;
 
 get_ses_fail:
@@ -2754,8 +2847,16 @@ void
 cifs_put_tcon(struct cifs_tcon *tcon)
 {
 	unsigned int xid;
-	struct cifs_ses *ses = tcon->ses;
+	struct cifs_ses *ses;
 
+	/*
+	 * IPC tcon share the lifetime of their session and are
+	 * destroyed in the session put function
+	 */
+	if (tcon == NULL || tcon->ipc)
+		return;
+
+	ses = tcon->ses;
 	cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
 	spin_lock(&cifs_tcp_ses_lock);
 	if (--tcon->tc_count > 0) {
@@ -3031,39 +3132,17 @@ get_dfs_path(const unsigned int xid, struct cifs_ses *ses, const char *old_path,
 	     const struct nls_table *nls_codepage, unsigned int *num_referrals,
 	     struct dfs_info3_param **referrals, int remap)
 {
-	char *temp_unc;
 	int rc = 0;
 
-	if (!ses->server->ops->tree_connect || !ses->server->ops->get_dfs_refer)
+	if (!ses->server->ops->get_dfs_refer)
 		return -ENOSYS;
 
 	*num_referrals = 0;
 	*referrals = NULL;
 
-	if (ses->ipc_tid == 0) {
-		temp_unc = kmalloc(2 /* for slashes */ +
-			strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
-				+ 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
-		if (temp_unc == NULL)
-			return -ENOMEM;
-		temp_unc[0] = '\\';
-		temp_unc[1] = '\\';
-		strcpy(temp_unc + 2, ses->serverName);
-		strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
-		rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
-						    nls_codepage);
-		cifs_dbg(FYI, "Tcon rc = %d ipc_tid = %d\n", rc, ses->ipc_tid);
-		kfree(temp_unc);
-	}
-	if (rc == 0)
-		rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
-						     referrals, num_referrals,
-						     nls_codepage, remap);
-	/*
-	 * BB - map targetUNCs to dfs_info3 structures, here or in
-	 * ses->server->ops->get_dfs_refer.
-	 */
-
+	rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
+					     referrals, num_referrals,
+					     nls_codepage, remap);
 	return rc;
 }
 
@@ -3828,7 +3907,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 		tcon->unix_ext = 0; /* server does not support them */
 
 	/* do not care if a following call succeed - informational */
-	if (!tcon->ipc && server->ops->qfs_tcon)
+	if (!tcon->pipe && server->ops->qfs_tcon)
 		server->ops->qfs_tcon(xid, tcon);
 
 	cifs_sb->wsize = server->ops->negotiate_wsize(tcon, volume_info);
@@ -3958,8 +4037,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
 }
 
 /*
- * Issue a TREE_CONNECT request. Note that for IPC$ shares, that the tcon
- * pointer may be NULL.
+ * Issue a TREE_CONNECT request.
  */
 int
 CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
@@ -3995,7 +4073,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 	pSMB->AndXCommand = 0xFF;
 	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
 	bcc_ptr = &pSMB->Password[0];
-	if (!tcon || (ses->server->sec_mode & SECMODE_USER)) {
+	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
 		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
 		*bcc_ptr = 0; /* password is null byte */
 		bcc_ptr++;              /* skip password */
@@ -4067,7 +4145,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			 0);
 
 	/* above now done in SendReceive */
-	if ((rc == 0) && (tcon != NULL)) {
+	if (rc == 0) {
 		bool is_unicode;
 
 		tcon->tidStatus = CifsGood;
@@ -4087,7 +4165,8 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 			if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
 			    (bcc_ptr[2] == 'C')) {
 				cifs_dbg(FYI, "IPC connection\n");
-				tcon->ipc = 1;
+				tcon->ipc = true;
+				tcon->pipe = true;
 			}
 		} else if (length == 2) {
 			if ((bcc_ptr[0] == 'A') && (bcc_ptr[1] == ':')) {
@@ -4114,9 +4193,6 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 		else
 			tcon->Flags = 0;
 		cifs_dbg(FYI, "Tcon flags: 0x%x\n", tcon->Flags);
-	} else if ((rc == 0) && tcon == NULL) {
-		/* all we need to save for IPC$ connection */
-		ses->ipc_tid = smb_buffer_response->Tid;
 	}
 
 	cifs_buf_release(smb_buffer);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ecb99079363a..8f9a8cc7cc62 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1049,7 +1049,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 	tcon->resource_id = CIFS_I(inode)->uniqueid;
 #endif
 
-	if (rc && tcon->ipc) {
+	if (rc && tcon->pipe) {
 		cifs_dbg(FYI, "ipc connection - fake read inode\n");
 		spin_lock(&inode->i_lock);
 		inode->i_mode |= S_IFDIR;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 7900aec7f92f..2943adc754e4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1258,8 +1258,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 	}
 
 	/* SMB2 TREE_CONNECT request must be called with TreeId == 0 */
-	if (tcon)
-		tcon->tid = 0;
+	tcon->tid = 0;
 
 	rc = smb2_plain_req_init(SMB2_TREE_CONNECT, tcon, (void **) &req,
 			     &total_len);
@@ -1268,15 +1267,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		return rc;
 	}
 
-	if (tcon == NULL) {
-		if ((ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
-			flags |= CIFS_TRANSFORM_REQ;
-
-		/* since no tcon, smb2_init can not do this, so do here */
-		req->sync_hdr.SessionId = ses->Suid;
-		if (ses->server->sign)
-			req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
-	} else if (encryption_required(tcon))
+	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
 	iov[0].iov_base = (char *)req;
@@ -1302,21 +1293,16 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
 		goto tcon_error_exit;
 	}
 
-	if (tcon == NULL) {
-		ses->ipc_tid = rsp->hdr.sync_hdr.TreeId;
-		goto tcon_exit;
-	}
-
 	switch (rsp->ShareType) {
 	case SMB2_SHARE_TYPE_DISK:
 		cifs_dbg(FYI, "connection to disk share\n");
 		break;
 	case SMB2_SHARE_TYPE_PIPE:
-		tcon->ipc = true;
+		tcon->pipe = true;
 		cifs_dbg(FYI, "connection to pipe share\n");
 		break;
 	case SMB2_SHARE_TYPE_PRINT:
-		tcon->ipc = true;
+		tcon->print = true;
 		cifs_dbg(FYI, "connection to printer\n");
 		break;
 	default:
@@ -1892,16 +1878,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	if (rc)
 		return rc;
 
-	if (use_ipc) {
-		if (ses->ipc_tid == 0) {
-			cifs_small_buf_release(req);
-			return -ENOTCONN;
-		}
-
-		cifs_dbg(FYI, "replacing tid 0x%x with IPC tid 0x%x\n",
-			 req->sync_hdr.TreeId, ses->ipc_tid);
-		req->sync_hdr.TreeId = ses->ipc_tid;
-	}
 	if (encryption_required(tcon))
 		flags |= CIFS_TRANSFORM_REQ;
 
@@ -2317,6 +2293,10 @@ void smb2_reconnect_server(struct work_struct *work)
 				tcon_exist = true;
 			}
 		}
+		if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
+			list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
+			tcon_exist = true;
+		}
 	}
 	/*
 	 * Get the reference to server struct to be sure that the last call of
-- 
2.12.3

RE: [PATCH v3 1/3] CIFS: make IPC a regular tcon

[Pavel Shilovskiy]

2018-01-24 4:46 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> * Remove ses->ipc_tid.
> * Make IPC$ regular tcon.
> * Add a direct pointer to it in ses->tcon_ipc.
> * Distinguish PIPE tcon from IPC tcon by adding a tcon->pipe flag. All
>   IPC tcons are pipes but not all pipes are IPC.
> * All TreeConnect functions now cannot take a NULL tcon object.
>
> The IPC tcon has the same lifetime as the session it belongs to. It is
> created when the session is created and destroyed when the session is
> destroyed.
>
> Since no mounts directly refer to the IPC tcon, its refcount should
> always be set to initialisation value (1). Thus we make sure
> cifs_put_tcon() skips it.
>
> If the mount request resulting in a new session being created requires
> encryption, try to require it too for IPC.
>
> * set SERVER_NAME_LENGTH to serverName actual size
>
> The maximum length of an ipv6 string representation is defined in
> INET6_ADDRSTRLEN as 45+1 for null but lets keep what we know works.
>
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h |  14 ++---
>  fs/cifs/cifssmb.c  |   7 +--
>  fs/cifs/connect.c  | 150 ++++++++++++++++++++++++++++++++++++++++-------------
>  fs/cifs/inode.c    |   2 +-
>  fs/cifs/smb2pdu.c  |  36 +++----------
>  5 files changed, 133 insertions(+), 76 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 678e638c1e69..48f7c197cd2d 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -64,8 +64,8 @@
>  #define RFC1001_NAME_LEN 15
>  #define RFC1001_NAME_LEN_WITH_NULL (RFC1001_NAME_LEN + 1)
>
> -/* currently length of NIP6_FMT */
> -#define SERVER_NAME_LENGTH 40
> +/* maximum length of ip addr as a string (including ipv6 and sctp) */
> +#define SERVER_NAME_LENGTH 80
>  #define SERVER_NAME_LEN_WITH_NULL     (SERVER_NAME_LENGTH + 1)
>
>  /* echo interval in seconds */
> @@ -833,12 +833,12 @@ static inline void cifs_set_net_ns(struct TCP_Server_Info *srv, struct net *net)
>  struct cifs_ses {
>         struct list_head smb_ses_list;
>         struct list_head tcon_list;
> +       struct cifs_tcon *tcon_ipc;
>         struct mutex session_mutex;
>         struct TCP_Server_Info *server; /* pointer to server info */
>         int ses_count;          /* reference counter */
>         enum statusEnum status;
>         unsigned overrideSecFlg;  /* if non-zero override global sec flags */
> -       __u32 ipc_tid;          /* special tid for connection to IPC share */
>         char *serverOS;         /* name of operating system underlying server */
>         char *serverNOS;        /* name of network operating system of server */
>         char *serverDomain;     /* security realm of server */
> @@ -846,8 +846,7 @@ struct cifs_ses {
>         kuid_t linux_uid;       /* overriding owner of files on the mount */
>         kuid_t cred_uid;        /* owner of credentials */
>         unsigned int capabilities;
> -       char serverName[SERVER_NAME_LEN_WITH_NULL * 2]; /* BB make bigger for
> -                               TCP names - will ipv6 and sctp addresses fit? */
> +       char serverName[SERVER_NAME_LEN_WITH_NULL];
>         char *user_name;        /* must not be null except during init of sess
>                                    and after mount option parsing we fill it */
>         char *domainName;
> @@ -942,7 +941,9 @@ struct cifs_tcon {
>         FILE_SYSTEM_DEVICE_INFO fsDevInfo;
>         FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
>         FILE_SYSTEM_UNIX_INFO fsUnixInfo;
> -       bool ipc:1;             /* set if connection to IPC$ eg for RPC/PIPES */
> +       bool ipc:1;   /* set if connection to IPC$ share (always also pipe) */
> +       bool pipe:1;  /* set if connection to pipe share */
> +       bool print:1; /* set if connection to printer share */
>         bool retry:1;
>         bool nocase:1;
>         bool seal:1;      /* transport encryption for this mounted share */
> @@ -955,7 +956,6 @@ struct cifs_tcon {
>         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 */
> -       bool print:1;           /* set if connection to printer share */
>         __le32 capabilities;
>         __u32 share_flags;
>         __u32 maximal_access;
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 49cf999f3d46..4e0922d24eb2 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4833,10 +4833,11 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>         *target_nodes = NULL;
>
>         cifs_dbg(FYI, "In GetDFSRefer the path %s\n", search_name);
> -       if (ses == NULL)
> +       if (ses == NULL || ses->tcon_ipc == NULL)
>                 return -ENODEV;
> +
>  getDFSRetry:
> -       rc = smb_init(SMB_COM_TRANSACTION2, 15, NULL, (void **) &pSMB,
> +       rc = smb_init(SMB_COM_TRANSACTION2, 15, ses->tcon_ipc, (void **) &pSMB,
>                       (void **) &pSMBr);
>         if (rc)
>                 return rc;
> @@ -4844,7 +4845,7 @@ CIFSGetDFSRefer(const unsigned int xid, struct cifs_ses *ses,
>         /* server pointer checked in called function,
>         but should never be null here anyway */
>         pSMB->hdr.Mid = get_next_mid(ses->server);
> -       pSMB->hdr.Tid = ses->ipc_tid;
> +       pSMB->hdr.Tid = ses->tcon_ipc->tid;
>         pSMB->hdr.Uid = ses->Suid;
>         if (ses->capabilities & CAP_STATUS32)
>                 pSMB->hdr.Flags2 |= SMBFLG2_ERR_STATUS;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 63c5d85fe25e..8b5e401f547a 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -354,11 +354,12 @@ cifs_reconnect(struct TCP_Server_Info *server)
>         list_for_each(tmp, &server->smb_ses_list) {
>                 ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
>                 ses->need_reconnect = true;
> -               ses->ipc_tid = 0;
>                 list_for_each(tmp2, &ses->tcon_list) {
>                         tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
>                         tcon->need_reconnect = true;
>                 }
> +               if (ses->tcon_ipc)
> +                       ses->tcon_ipc->need_reconnect = true;
>         }
>         spin_unlock(&cifs_tcp_ses_lock);
>
> @@ -2426,6 +2427,93 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
>         return 1;
>  }
>
> +/**
> + * cifs_setup_ipc - helper to setup the IPC tcon for the session
> + *
> + * A new IPC connection is made and stored in the session
> + * tcon_ipc. The IPC tcon has the same lifetime as the session.
> + */
> +static int
> +cifs_setup_ipc(struct cifs_ses *ses, struct smb_vol *volume_info)
> +{
> +       int rc = 0, xid;
> +       struct cifs_tcon *tcon;
> +       struct nls_table *nls_codepage;
> +       char unc[SERVER_NAME_LENGTH + sizeof("//x/IPC$")] = {0};
> +       bool seal = false;
> +
> +       /*
> +        * If the mount request that resulted in the creation of the
> +        * session requires encryption, force IPC to be encrypted too.
> +        */
> +       if (volume_info->seal) {
> +               if (ses->server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)
> +                       seal = true;
> +               else {
> +                       cifs_dbg(VFS,
> +                                "IPC: server doesn't support encryption\n");
> +                       return -EOPNOTSUPP;
> +               }
> +       }
> +
> +       tcon = tconInfoAlloc();
> +       if (tcon == NULL)
> +               return -ENOMEM;
> +
> +       snprintf(unc, sizeof(unc), "\\\\%s\\IPC$", ses->serverName);
> +
> +       /* cannot fail */
> +       nls_codepage = load_nls_default();
> +
> +       xid = get_xid();
> +       tcon->ses = ses;
> +       tcon->ipc = true;
> +       tcon->seal = seal;
> +       rc = ses->server->ops->tree_connect(xid, ses, unc, tcon, nls_codepage);
> +       free_xid(xid);
> +
> +       if (rc) {
> +               cifs_dbg(VFS, "failed to connect to IPC (rc=%d)\n", rc);
> +               tconInfoFree(tcon);
> +               goto out;
> +       }
> +
> +       cifs_dbg(FYI, "IPC tcon rc = %d ipc tid = %d\n", rc, tcon->tid);
> +
> +       ses->tcon_ipc = tcon;
> +out:
> +       unload_nls(nls_codepage);
> +       return rc;
> +}
> +
> +/**
> + * cifs_free_ipc - helper to release the session IPC tcon
> + *
> + * Needs to be called everytime a session is destroyed
> + */
> +static int
> +cifs_free_ipc(struct cifs_ses *ses)
> +{
> +       int rc = 0, xid;
> +       struct cifs_tcon *tcon = ses->tcon_ipc;
> +
> +       if (tcon == NULL)
> +               return 0;
> +
> +       if (ses->server->ops->tree_disconnect) {
> +               xid = get_xid();
> +               rc = ses->server->ops->tree_disconnect(xid, tcon);
> +               free_xid(xid);
> +       }
> +
> +       if (rc)
> +               cifs_dbg(FYI, "failed to disconnect IPC tcon (rc=%d)\n", rc);
> +
> +       tconInfoFree(tcon);
> +       ses->tcon_ipc = NULL;
> +       return rc;
> +}
> +
>  static struct cifs_ses *
>  cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
>  {
> @@ -2466,6 +2554,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>                 ses->status = CifsExiting;
>         spin_unlock(&cifs_tcp_ses_lock);
>
> +       cifs_free_ipc(ses);
> +
>         if (ses->status == CifsExiting && server->ops->logoff) {
>                 xid = get_xid();
>                 rc = server->ops->logoff(xid, ses);
> @@ -2710,6 +2800,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
>         spin_unlock(&cifs_tcp_ses_lock);
>
>         free_xid(xid);
> +
> +       cifs_setup_ipc(ses, volume_info);
> +
>         return ses;
>
>  get_ses_fail:
> @@ -2754,8 +2847,16 @@ void
>  cifs_put_tcon(struct cifs_tcon *tcon)
>  {
>         unsigned int xid;
> -       struct cifs_ses *ses = tcon->ses;
> +       struct cifs_ses *ses;
>
> +       /*
> +        * IPC tcon share the lifetime of their session and are
> +        * destroyed in the session put function
> +        */
> +       if (tcon == NULL || tcon->ipc)
> +               return;
> +
> +       ses = tcon->ses;
>         cifs_dbg(FYI, "%s: tc_count=%d\n", __func__, tcon->tc_count);
>         spin_lock(&cifs_tcp_ses_lock);
>         if (--tcon->tc_count > 0) {
> @@ -3031,39 +3132,17 @@ get_dfs_path(const unsigned int xid, struct cifs_ses *ses, const char *old_path,
>              const struct nls_table *nls_codepage, unsigned int *num_referrals,
>              struct dfs_info3_param **referrals, int remap)
>  {
> -       char *temp_unc;
>         int rc = 0;
>
> -       if (!ses->server->ops->tree_connect || !ses->server->ops->get_dfs_refer)
> +       if (!ses->server->ops->get_dfs_refer)
>                 return -ENOSYS;
>
>         *num_referrals = 0;
>         *referrals = NULL;
>
> -       if (ses->ipc_tid == 0) {
> -               temp_unc = kmalloc(2 /* for slashes */ +
> -                       strnlen(ses->serverName, SERVER_NAME_LEN_WITH_NULL * 2)
> -                               + 1 + 4 /* slash IPC$ */ + 2, GFP_KERNEL);
> -               if (temp_unc == NULL)
> -                       return -ENOMEM;
> -               temp_unc[0] = '\\';
> -               temp_unc[1] = '\\';
> -               strcpy(temp_unc + 2, ses->serverName);
> -               strcpy(temp_unc + 2 + strlen(ses->serverName), "\\IPC$");
> -               rc = ses->server->ops->tree_connect(xid, ses, temp_unc, NULL,
> -                                                   nls_codepage);
> -               cifs_dbg(FYI, "Tcon rc = %d ipc_tid = %d\n", rc, ses->ipc_tid);
> -               kfree(temp_unc);
> -       }
> -       if (rc == 0)
> -               rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
> -                                                    referrals, num_referrals,
> -                                                    nls_codepage, remap);
> -       /*
> -        * BB - map targetUNCs to dfs_info3 structures, here or in
> -        * ses->server->ops->get_dfs_refer.
> -        */
> -
> +       rc = ses->server->ops->get_dfs_refer(xid, ses, old_path,
> +                                            referrals, num_referrals,
> +                                            nls_codepage, remap);
>         return rc;
>  }
>
> @@ -3828,7 +3907,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
>                 tcon->unix_ext = 0; /* server does not support them */
>
>         /* do not care if a following call succeed - informational */
> -       if (!tcon->ipc && server->ops->qfs_tcon)
> +       if (!tcon->pipe && server->ops->qfs_tcon)
>                 server->ops->qfs_tcon(xid, tcon);
>
>         cifs_sb->wsize = server->ops->negotiate_wsize(tcon, volume_info);
> @@ -3958,8 +4037,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
>  }
>
>  /*
> - * Issue a TREE_CONNECT request. Note that for IPC$ shares, that the tcon
> - * pointer may be NULL.
> + * Issue a TREE_CONNECT request.
>   */
>  int
>  CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> @@ -3995,7 +4073,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>         pSMB->AndXCommand = 0xFF;
>         pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
>         bcc_ptr = &pSMB->Password[0];
> -       if (!tcon || (ses->server->sec_mode & SECMODE_USER)) {
> +       if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
>                 pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
>                 *bcc_ptr = 0; /* password is null byte */
>                 bcc_ptr++;              /* skip password */
> @@ -4067,7 +4145,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                          0);
>
>         /* above now done in SendReceive */
> -       if ((rc == 0) && (tcon != NULL)) {
> +       if (rc == 0) {
>                 bool is_unicode;
>
>                 tcon->tidStatus = CifsGood;
> @@ -4087,7 +4165,8 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                         if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
>                             (bcc_ptr[2] == 'C')) {
>                                 cifs_dbg(FYI, "IPC connection\n");
> -                               tcon->ipc = 1;
> +                               tcon->ipc = true;
> +                               tcon->pipe = true;
>                         }
>                 } else if (length == 2) {
>                         if ((bcc_ptr[0] == 'A') && (bcc_ptr[1] == ':')) {
> @@ -4114,9 +4193,6 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>                 else
>                         tcon->Flags = 0;
>                 cifs_dbg(FYI, "Tcon flags: 0x%x\n", tcon->Flags);
> -       } else if ((rc == 0) && tcon == NULL) {
> -               /* all we need to save for IPC$ connection */
> -               ses->ipc_tid = smb_buffer_response->Tid;
>         }
>
>         cifs_buf_release(smb_buffer);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index ecb99079363a..8f9a8cc7cc62 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1049,7 +1049,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
>         tcon->resource_id = CIFS_I(inode)->uniqueid;
>  #endif
>
> -       if (rc && tcon->ipc) {
> +       if (rc && tcon->pipe) {
>                 cifs_dbg(FYI, "ipc connection - fake read inode\n");
>                 spin_lock(&inode->i_lock);
>                 inode->i_mode |= S_IFDIR;
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 7900aec7f92f..2943adc754e4 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1258,8 +1258,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>         }
>
>         /* SMB2 TREE_CONNECT request must be called with TreeId == 0 */
> -       if (tcon)
> -               tcon->tid = 0;
> +       tcon->tid = 0;
>
>         rc = smb2_plain_req_init(SMB2_TREE_CONNECT, tcon, (void **) &req,
>                              &total_len);
> @@ -1268,15 +1267,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>                 return rc;
>         }
>
> -       if (tcon == NULL) {
> -               if ((ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA))
> -                       flags |= CIFS_TRANSFORM_REQ;
> -
> -               /* since no tcon, smb2_init can not do this, so do here */
> -               req->sync_hdr.SessionId = ses->Suid;
> -               if (ses->server->sign)
> -                       req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
> -       } else if (encryption_required(tcon))
> +       if (encryption_required(tcon))
>                 flags |= CIFS_TRANSFORM_REQ;
>
>         iov[0].iov_base = (char *)req;
> @@ -1302,21 +1293,16 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree,
>                 goto tcon_error_exit;
>         }
>
> -       if (tcon == NULL) {
> -               ses->ipc_tid = rsp->hdr.sync_hdr.TreeId;
> -               goto tcon_exit;
> -       }
> -
>         switch (rsp->ShareType) {
>         case SMB2_SHARE_TYPE_DISK:
>                 cifs_dbg(FYI, "connection to disk share\n");
>                 break;
>         case SMB2_SHARE_TYPE_PIPE:
> -               tcon->ipc = true;
> +               tcon->pipe = true;
>                 cifs_dbg(FYI, "connection to pipe share\n");
>                 break;
>         case SMB2_SHARE_TYPE_PRINT:
> -               tcon->ipc = true;
> +               tcon->print = true;
>                 cifs_dbg(FYI, "connection to printer\n");
>                 break;
>         default:
> @@ -1892,16 +1878,6 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>         if (rc)
>                 return rc;
>
> -       if (use_ipc) {
> -               if (ses->ipc_tid == 0) {
> -                       cifs_small_buf_release(req);
> -                       return -ENOTCONN;
> -               }
> -
> -               cifs_dbg(FYI, "replacing tid 0x%x with IPC tid 0x%x\n",
> -                        req->sync_hdr.TreeId, ses->ipc_tid);
> -               req->sync_hdr.TreeId = ses->ipc_tid;
> -       }
>         if (encryption_required(tcon))
>                 flags |= CIFS_TRANSFORM_REQ;
>
> @@ -2317,6 +2293,10 @@ void smb2_reconnect_server(struct work_struct *work)
>                                 tcon_exist = true;
>                         }
>                 }
> +               if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
> +                       list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
> +                       tcon_exist = true;
> +               }
>         }
>         /*
>          * Get the reference to server struct to be sure that the last call of
> --
> 2.12.3
>
> --
> 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

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

--
Best regards,
Pavel Shilovsky

[PATCH v3 2/3] CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

[Aurelien Aptel]

Since IPC now has a tcon object, the caller can just pass it. This
allows domain-based DFS requests to work with smb2+.

Link: https://bugzilla.samba.org/show_bug.cgi?id=12917
Fixes: 9d49640a21bf ("CIFS: implement get_dfs_refer for SMB2+")
Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/smb2file.c  |  2 +-
 fs/cifs/smb2ops.c   | 53 ++++++++++++++++++++++-------------------------------
 fs/cifs/smb2pdu.c   |  4 +---
 fs/cifs/smb2proto.h |  3 +--
 4 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index b4b1f0305f29..12af5dba742b 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -74,7 +74,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
 		nr_ioctl_req.Reserved = 0;
 		rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
 			fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
 			NULL, NULL /* no return info */);
 		if (rc == -EOPNOTSUPP) {
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index e3393ff5d458..eb68e2fcc500 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -293,7 +293,6 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 			FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL /* no data input */, 0 /* no data input */,
 			(char **)&out_buf, &ret_data_len);
 	if (rc != 0)
@@ -792,7 +791,6 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
-			false /* use_ipc */,
 			NULL, 0 /* no input */,
 			(char **)&res_key, &ret_data_len);
 
@@ -858,8 +856,7 @@ smb2_copychunk_range(const unsigned int xid,
 		/* Request server copy to target from src identified by key */
 		rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
-			true /* is_fsctl */, false /* use_ipc */,
-			(char *)pcchunk,
+			true /* is_fsctl */, (char *)pcchunk,
 			sizeof(struct copychunk_ioctl),	(char **)&retbuf,
 			&ret_data_len);
 		if (rc == 0) {
@@ -1020,7 +1017,7 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
-			true /* is_fctl */, false /* use_ipc */,
+			true /* is_fctl */,
 			&setsparse, 1, NULL, NULL);
 	if (rc) {
 		tcon->broken_sparse_sup = true;
@@ -1091,7 +1088,7 @@ smb2_duplicate_extents(const unsigned int xid,
 	rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
 			trgtfile->fid.volatile_fid,
 			FSCTL_DUPLICATE_EXTENTS_TO_FILE,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&dup_ext_buf,
 			sizeof(struct duplicate_extents_to_file),
 			NULL,
@@ -1126,7 +1123,7 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
 	return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SET_INTEGRITY_INFORMATION,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			(char *)&integr_info,
 			sizeof(struct fsctl_set_integrity_information_req),
 			NULL,
@@ -1146,7 +1143,7 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid,
 			FSCTL_SRV_ENUMERATE_SNAPSHOTS,
-			true /* is_fsctl */, false /* use_ipc */,
+			true /* is_fsctl */,
 			NULL, 0 /* no input data */,
 			(char **)&retbuf,
 			&ret_data_len);
@@ -1365,16 +1362,20 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
 
 	/*
-	 * Use any tcon from the current session. Here, the first one.
+	 * Try to use the IPC tcon, otherwise just use any
 	 */
-	spin_lock(&cifs_tcp_ses_lock);
-	tcon = list_first_entry_or_null(&ses->tcon_list, struct cifs_tcon,
-					tcon_list);
-	if (tcon)
-		tcon->tc_count++;
-	spin_unlock(&cifs_tcp_ses_lock);
+	tcon = ses->tcon_ipc;
+	if (tcon == NULL) {
+		spin_lock(&cifs_tcp_ses_lock);
+		tcon = list_first_entry_or_null(&ses->tcon_list,
+						struct cifs_tcon,
+						tcon_list);
+		if (tcon)
+			tcon->tc_count++;
+		spin_unlock(&cifs_tcp_ses_lock);
+	}
 
-	if (!tcon) {
+	if (tcon == NULL) {
 		cifs_dbg(VFS, "session %p has no tcon available for a dfs referral request\n",
 			 ses);
 		rc = -ENOTCONN;
@@ -1403,20 +1404,11 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
 
 	do {
-		/* try first with IPC */
 		rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 				FSCTL_DFS_GET_REFERRALS,
-				true /* is_fsctl */, true /* use_ipc */,
+				true /* is_fsctl */,
 				(char *)dfs_req, dfs_req_size,
 				(char **)&dfs_rsp, &dfs_rsp_size);
-		if (rc == -ENOTCONN) {
-			/* try with normal tcon */
-			rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
-					FSCTL_DFS_GET_REFERRALS,
-					true /* is_fsctl */, false /*use_ipc*/,
-					(char *)dfs_req, dfs_req_size,
-					(char **)&dfs_rsp, &dfs_rsp_size);
-		}
 	} while (rc == -EAGAIN);
 
 	if (rc) {
@@ -1435,7 +1427,8 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
 	}
 
  out:
-	if (tcon) {
+	if (tcon && !tcon->ipc) {
+		/* ipc tcons are not refcounted */
 		spin_lock(&cifs_tcp_ses_lock);
 		tcon->tc_count--;
 		spin_unlock(&cifs_tcp_ses_lock);
@@ -1727,8 +1720,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
@@ -1762,8 +1754,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
 			cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
-			true /* is_fctl */, false /* use_ipc */,
-			(char *)&fsctl_buf,
+			true /* is_fctl */, (char *)&fsctl_buf,
 			sizeof(struct file_zero_data_information), NULL, NULL);
 	free_xid(xid);
 	return rc;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2943adc754e4..17b7f3aed195 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -680,7 +680,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
 
 	rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
 		FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
-		false /* use_ipc */,
 		(char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req),
 		(char **)&pneg_rsp, &rsplen);
 
@@ -1841,7 +1840,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
  */
 int
 SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
-	   u64 volatile_fid, u32 opcode, bool is_fsctl, bool use_ipc,
+	   u64 volatile_fid, u32 opcode, bool is_fsctl,
 	   char *in_data, u32 indatalen,
 	   char **out_data, u32 *plen /* returned data len */)
 {
@@ -2006,7 +2005,6 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
 
 	rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
 			FSCTL_SET_COMPRESSION, true /* is_fsctl */,
-			false /* use_ipc */,
 			(char *)&fsctl_input /* data input */,
 			2 /* in data len */, &ret_data /* out data */, NULL);
 
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index e9ab5227e7a8..05287b01f596 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -125,8 +125,7 @@ extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms,
 		     struct smb2_err_rsp **err_buf);
 extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
 		     u64 persistent_fid, u64 volatile_fid, u32 opcode,
-		     bool is_fsctl, bool use_ipc,
-		     char *in_data, u32 indatalen,
+		     bool is_fsctl, char *in_data, u32 indatalen,
 		     char **out_data, u32 *plen /* returned data len */);
 extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 		      u64 persistent_file_id, u64 volatile_file_id);
-- 
2.12.3

RE: [PATCH v3 2/3] CIFS: use tcon_ipc instead of use_ipc parameter of SMB2_ioctl

[Pavel Shilovskiy]

2018-01-24 4:46 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> Since IPC now has a tcon object, the caller can just pass it. This
> allows domain-based DFS requests to work with smb2+.
>
> Link: https://bugzilla.samba.org/show_bug.cgi?id=12917
> Fixes: 9d49640a21bf ("CIFS: implement get_dfs_refer for SMB2+")
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/smb2file.c  |  2 +-
>  fs/cifs/smb2ops.c   | 53 ++++++++++++++++++++++-------------------------------
>  fs/cifs/smb2pdu.c   |  4 +---
>  fs/cifs/smb2proto.h |  3 +--
>  4 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index b4b1f0305f29..12af5dba742b 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -74,7 +74,7 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms,
>                 nr_ioctl_req.Reserved = 0;
>                 rc = SMB2_ioctl(xid, oparms->tcon, fid->persistent_fid,
>                         fid->volatile_fid, FSCTL_LMR_REQUEST_RESILIENCY,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&nr_ioctl_req, sizeof(nr_ioctl_req),
>                         NULL, NULL /* no return info */);
>                 if (rc == -EOPNOTSUPP) {
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index e3393ff5d458..eb68e2fcc500 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -293,7 +293,6 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
>
>         rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>                         FSCTL_QUERY_NETWORK_INTERFACE_INFO, true /* is_fsctl */,
> -                       false /* use_ipc */,
>                         NULL /* no data input */, 0 /* no data input */,
>                         (char **)&out_buf, &ret_data_len);
>         if (rc != 0)
> @@ -792,7 +791,6 @@ SMB2_request_res_key(const unsigned int xid, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>                         FSCTL_SRV_REQUEST_RESUME_KEY, true /* is_fsctl */,
> -                       false /* use_ipc */,
>                         NULL, 0 /* no input */,
>                         (char **)&res_key, &ret_data_len);
>
> @@ -858,8 +856,7 @@ smb2_copychunk_range(const unsigned int xid,
>                 /* Request server copy to target from src identified by key */
>                 rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>                         trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
> -                       true /* is_fsctl */, false /* use_ipc */,
> -                       (char *)pcchunk,
> +                       true /* is_fsctl */, (char *)pcchunk,
>                         sizeof(struct copychunk_ioctl), (char **)&retbuf,
>                         &ret_data_len);
>                 if (rc == 0) {
> @@ -1020,7 +1017,7 @@ static bool smb2_set_sparse(const unsigned int xid, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid, FSCTL_SET_SPARSE,
> -                       true /* is_fctl */, false /* use_ipc */,
> +                       true /* is_fctl */,
>                         &setsparse, 1, NULL, NULL);
>         if (rc) {
>                 tcon->broken_sparse_sup = true;
> @@ -1091,7 +1088,7 @@ smb2_duplicate_extents(const unsigned int xid,
>         rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>                         trgtfile->fid.volatile_fid,
>                         FSCTL_DUPLICATE_EXTENTS_TO_FILE,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&dup_ext_buf,
>                         sizeof(struct duplicate_extents_to_file),
>                         NULL,
> @@ -1126,7 +1123,7 @@ smb3_set_integrity(const unsigned int xid, struct cifs_tcon *tcon,
>         return SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid,
>                         FSCTL_SET_INTEGRITY_INFORMATION,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         (char *)&integr_info,
>                         sizeof(struct fsctl_set_integrity_information_req),
>                         NULL,
> @@ -1146,7 +1143,7 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid,
>                         FSCTL_SRV_ENUMERATE_SNAPSHOTS,
> -                       true /* is_fsctl */, false /* use_ipc */,
> +                       true /* is_fsctl */,
>                         NULL, 0 /* no input data */,
>                         (char **)&retbuf,
>                         &ret_data_len);
> @@ -1365,16 +1362,20 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>         cifs_dbg(FYI, "smb2_get_dfs_refer path <%s>\n", search_name);
>
>         /*
> -        * Use any tcon from the current session. Here, the first one.
> +        * Try to use the IPC tcon, otherwise just use any
>          */
> -       spin_lock(&cifs_tcp_ses_lock);
> -       tcon = list_first_entry_or_null(&ses->tcon_list, struct cifs_tcon,
> -                                       tcon_list);
> -       if (tcon)
> -               tcon->tc_count++;
> -       spin_unlock(&cifs_tcp_ses_lock);
> +       tcon = ses->tcon_ipc;
> +       if (tcon == NULL) {
> +               spin_lock(&cifs_tcp_ses_lock);
> +               tcon = list_first_entry_or_null(&ses->tcon_list,
> +                                               struct cifs_tcon,
> +                                               tcon_list);
> +               if (tcon)
> +                       tcon->tc_count++;
> +               spin_unlock(&cifs_tcp_ses_lock);
> +       }
>
> -       if (!tcon) {
> +       if (tcon == NULL) {
>                 cifs_dbg(VFS, "session %p has no tcon available for a dfs referral request\n",
>                          ses);
>                 rc = -ENOTCONN;
> @@ -1403,20 +1404,11 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>         memcpy(dfs_req->RequestFileName, utf16_path, utf16_path_len);
>
>         do {
> -               /* try first with IPC */
>                 rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>                                 FSCTL_DFS_GET_REFERRALS,
> -                               true /* is_fsctl */, true /* use_ipc */,
> +                               true /* is_fsctl */,
>                                 (char *)dfs_req, dfs_req_size,
>                                 (char **)&dfs_rsp, &dfs_rsp_size);
> -               if (rc == -ENOTCONN) {
> -                       /* try with normal tcon */
> -                       rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
> -                                       FSCTL_DFS_GET_REFERRALS,
> -                                       true /* is_fsctl */, false /*use_ipc*/,
> -                                       (char *)dfs_req, dfs_req_size,
> -                                       (char **)&dfs_rsp, &dfs_rsp_size);
> -               }
>         } while (rc == -EAGAIN);
>
>         if (rc) {
> @@ -1435,7 +1427,8 @@ smb2_get_dfs_refer(const unsigned int xid, struct cifs_ses *ses,
>         }
>
>   out:
> -       if (tcon) {
> +       if (tcon && !tcon->ipc) {
> +               /* ipc tcons are not refcounted */
>                 spin_lock(&cifs_tcp_ses_lock);
>                 tcon->tc_count--;
>                 spin_unlock(&cifs_tcp_ses_lock);
> @@ -1727,8 +1720,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> -                       true /* is_fctl */, false /* use_ipc */,
> -                       (char *)&fsctl_buf,
> +                       true /* is_fctl */, (char *)&fsctl_buf,
>                         sizeof(struct file_zero_data_information), NULL, NULL);
>         free_xid(xid);
>         return rc;
> @@ -1762,8 +1754,7 @@ static long smb3_punch_hole(struct file *file, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
>                         cfile->fid.volatile_fid, FSCTL_SET_ZERO_DATA,
> -                       true /* is_fctl */, false /* use_ipc */,
> -                       (char *)&fsctl_buf,
> +                       true /* is_fctl */, (char *)&fsctl_buf,
>                         sizeof(struct file_zero_data_information), NULL, NULL);
>         free_xid(xid);
>         return rc;
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 2943adc754e4..17b7f3aed195 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -680,7 +680,6 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
>
>         rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
>                 FSCTL_VALIDATE_NEGOTIATE_INFO, true /* is_fsctl */,
> -               false /* use_ipc */,
>                 (char *)&vneg_inbuf, sizeof(struct validate_negotiate_info_req),
>                 (char **)&pneg_rsp, &rsplen);
>
> @@ -1841,7 +1840,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
>   */
>  int
>  SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
> -          u64 volatile_fid, u32 opcode, bool is_fsctl, bool use_ipc,
> +          u64 volatile_fid, u32 opcode, bool is_fsctl,
>            char *in_data, u32 indatalen,
>            char **out_data, u32 *plen /* returned data len */)
>  {
> @@ -2006,7 +2005,6 @@ SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon,
>
>         rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid,
>                         FSCTL_SET_COMPRESSION, true /* is_fsctl */,
> -                       false /* use_ipc */,
>                         (char *)&fsctl_input /* data input */,
>                         2 /* in data len */, &ret_data /* out data */, NULL);
>
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index e9ab5227e7a8..05287b01f596 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -125,8 +125,7 @@ extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms,
>                      struct smb2_err_rsp **err_buf);
>  extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon,
>                      u64 persistent_fid, u64 volatile_fid, u32 opcode,
> -                    bool is_fsctl, bool use_ipc,
> -                    char *in_data, u32 indatalen,
> +                    bool is_fsctl, char *in_data, u32 indatalen,
>                      char **out_data, u32 *plen /* returned data len */);
>  extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
>                       u64 persistent_file_id, u64 volatile_file_id);
> --
> 2.12.3
>
> --
> 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

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

--
Best regards,
Pavel Shilovsky

[PATCH v3 3/3] CIFS: dump IPC tcon in debug proc file

[Aurelien Aptel]

dump it as first share with an "IPC: " prefix.

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

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 05be9b47eb0c..f491340f32ad 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -110,6 +110,32 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
 }
 
 #ifdef CONFIG_PROC_FS
+static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
+{
+	__u32 dev_type = le32_to_cpu(tcon->fsDevInfo.DeviceType);
+
+	seq_printf(m, "%s Mounts: %d ", tcon->treeName, tcon->tc_count);
+	if (tcon->nativeFileSystem)
+		seq_printf(m, "Type: %s ", tcon->nativeFileSystem);
+	seq_printf(m, "DevInfo: 0x%x Attributes: 0x%x\n\tPathComponentMax: %d Status: %d",
+		   le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics),
+		   le32_to_cpu(tcon->fsAttrInfo.Attributes),
+		   le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
+		   tcon->tidStatus);
+	if (dev_type == FILE_DEVICE_DISK)
+		seq_puts(m, " type: DISK ");
+	else if (dev_type == FILE_DEVICE_CD_ROM)
+		seq_puts(m, " type: CDROM ");
+	else
+		seq_printf(m, " type: %d ", dev_type);
+	if (tcon->ses->server->ops->dump_share_caps)
+		tcon->ses->server->ops->dump_share_caps(m, tcon);
+
+	if (tcon->need_reconnect)
+		seq_puts(m, "\tDISCONNECTED ");
+	seq_putc(m, '\n');
+}
+
 static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 {
 	struct list_head *tmp1, *tmp2, *tmp3;
@@ -118,7 +144,6 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
 	int i, j;
-	__u32 dev_type;
 
 	seq_puts(m,
 		    "Display Internal CIFS Data Structures for Debugging\n"
@@ -260,35 +285,19 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 
 			seq_puts(m, "\n\tShares:");
 			j = 0;
+
+			seq_printf(m, "\n\t%d) IPC: ", j);
+			if (ses->tcon_ipc)
+				cifs_debug_tcon(m, ses->tcon_ipc);
+			else
+				seq_puts(m, "none\n");
+
 			list_for_each(tmp3, &ses->tcon_list) {
 				tcon = list_entry(tmp3, struct cifs_tcon,
 						  tcon_list);
 				++j;
-				dev_type = le32_to_cpu(tcon->fsDevInfo.DeviceType);
-				seq_printf(m, "\n\t%d) %s Mounts: %d ", j,
-					   tcon->treeName, tcon->tc_count);
-				if (tcon->nativeFileSystem) {
-					seq_printf(m, "Type: %s ",
-						   tcon->nativeFileSystem);
-				}
-				seq_printf(m, "DevInfo: 0x%x Attributes: 0x%x"
-					"\n\tPathComponentMax: %d Status: %d",
-					le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics),
-					le32_to_cpu(tcon->fsAttrInfo.Attributes),
-					le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
-					tcon->tidStatus);
-				if (dev_type == FILE_DEVICE_DISK)
-					seq_puts(m, " type: DISK ");
-				else if (dev_type == FILE_DEVICE_CD_ROM)
-					seq_puts(m, " type: CDROM ");
-				else
-					seq_printf(m, " type: %d ", dev_type);
-				if (server->ops->dump_share_caps)
-					server->ops->dump_share_caps(m, tcon);
-
-				if (tcon->need_reconnect)
-					seq_puts(m, "\tDISCONNECTED ");
-				seq_putc(m, '\n');
+				seq_printf(m, "\n\t%d) ", j);
+				cifs_debug_tcon(m, tcon);
 			}
 
 			seq_puts(m, "\n\tMIDs:\n");
-- 
2.12.3

RE: [PATCH v3 3/3] CIFS: dump IPC tcon in debug proc file

[Pavel Shilovskiy]

2018-01-24 4:46 GMT-08:00 Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>:
> dump it as first share with an "IPC: " prefix.
>
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/cifs_debug.c | 61 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 26 deletions(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 05be9b47eb0c..f491340f32ad 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -110,6 +110,32 @@ void cifs_dump_mids(struct TCP_Server_Info *server)
>  }
>
>  #ifdef CONFIG_PROC_FS
> +static void cifs_debug_tcon(struct seq_file *m, struct cifs_tcon *tcon)
> +{
> +       __u32 dev_type = le32_to_cpu(tcon->fsDevInfo.DeviceType);
> +
> +       seq_printf(m, "%s Mounts: %d ", tcon->treeName, tcon->tc_count);
> +       if (tcon->nativeFileSystem)
> +               seq_printf(m, "Type: %s ", tcon->nativeFileSystem);
> +       seq_printf(m, "DevInfo: 0x%x Attributes: 0x%x\n\tPathComponentMax: %d Status: %d",
> +                  le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics),
> +                  le32_to_cpu(tcon->fsAttrInfo.Attributes),
> +                  le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
> +                  tcon->tidStatus);
> +       if (dev_type == FILE_DEVICE_DISK)
> +               seq_puts(m, " type: DISK ");
> +       else if (dev_type == FILE_DEVICE_CD_ROM)
> +               seq_puts(m, " type: CDROM ");
> +       else
> +               seq_printf(m, " type: %d ", dev_type);
> +       if (tcon->ses->server->ops->dump_share_caps)
> +               tcon->ses->server->ops->dump_share_caps(m, tcon);
> +
> +       if (tcon->need_reconnect)
> +               seq_puts(m, "\tDISCONNECTED ");
> +       seq_putc(m, '\n');
> +}
> +
>  static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
>  {
>         struct list_head *tmp1, *tmp2, *tmp3;
> @@ -118,7 +144,6 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
>         struct cifs_ses *ses;
>         struct cifs_tcon *tcon;
>         int i, j;
> -       __u32 dev_type;
>
>         seq_puts(m,
>                     "Display Internal CIFS Data Structures for Debugging\n"
> @@ -260,35 +285,19 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
>
>                         seq_puts(m, "\n\tShares:");
>                         j = 0;
> +
> +                       seq_printf(m, "\n\t%d) IPC: ", j);
> +                       if (ses->tcon_ipc)
> +                               cifs_debug_tcon(m, ses->tcon_ipc);
> +                       else
> +                               seq_puts(m, "none\n");
> +
>                         list_for_each(tmp3, &ses->tcon_list) {
>                                 tcon = list_entry(tmp3, struct cifs_tcon,
>                                                   tcon_list);
>                                 ++j;
> -                               dev_type = le32_to_cpu(tcon->fsDevInfo.DeviceType);
> -                               seq_printf(m, "\n\t%d) %s Mounts: %d ", j,
> -                                          tcon->treeName, tcon->tc_count);
> -                               if (tcon->nativeFileSystem) {
> -                                       seq_printf(m, "Type: %s ",
> -                                                  tcon->nativeFileSystem);
> -                               }
> -                               seq_printf(m, "DevInfo: 0x%x Attributes: 0x%x"
> -                                       "\n\tPathComponentMax: %d Status: %d",
> -                                       le32_to_cpu(tcon->fsDevInfo.DeviceCharacteristics),
> -                                       le32_to_cpu(tcon->fsAttrInfo.Attributes),
> -                                       le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength),
> -                                       tcon->tidStatus);
> -                               if (dev_type == FILE_DEVICE_DISK)
> -                                       seq_puts(m, " type: DISK ");
> -                               else if (dev_type == FILE_DEVICE_CD_ROM)
> -                                       seq_puts(m, " type: CDROM ");
> -                               else
> -                                       seq_printf(m, " type: %d ", dev_type);
> -                               if (server->ops->dump_share_caps)
> -                                       server->ops->dump_share_caps(m, tcon);
> -
> -                               if (tcon->need_reconnect)
> -                                       seq_puts(m, "\tDISCONNECTED ");
> -                               seq_putc(m, '\n');
> +                               seq_printf(m, "\n\t%d) ", j);
> +                               cifs_debug_tcon(m, tcon);
>                         }
>
>                         seq_puts(m, "\n\tMIDs:\n");
> --
> 2.12.3
>
> --
> 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

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

--
Best regards,
Pavel Shilovsky