[PATCH 0/8] Clean up MF-Symlink code and add readlink support for DFS shares

[PATCH 0/8] Clean up MF-Symlink code and add readlink support for DFS shares

[Sachin Prabhu]

The following set of patches was created as a result of an error
reported by Red Hat QA which requires code to allow us to call
readlink on dfs shares.

Along with a fix for this issue, it appeared to be a good time to
clean up the M-F symlink code to make it protocol agnostic as well
as fix a couple of bugs in it.

Sachin Prabhu (8):
  cifs: We do not drop reference to tlink in  CIFSCheckMFSymlink()
  cifs: Rename and cleanup open_query_close_cifs_symlink()
  cifs: Rename MF symlink function names
  cifs: use protocol specific call for query_mf_symlink()
  cifs: Add create MFSymlinks to protocol ops struct
  cifs: Re-order M-F Symlink code
  cifs: move unix extension call to cifs_query_symlink()
  cifs: Add support for readlink on dfs shares under posix extensions

 fs/cifs/cifsglob.h  |   8 +-
 fs/cifs/cifsproto.h |  20 ++--
 fs/cifs/inode.c     |  10 +-
 fs/cifs/link.c      | 315 ++++++++++++++++++++++++----------------------------
 fs/cifs/readdir.c   |   2 +-
 fs/cifs/smb1ops.c   |  55 +++++++--
 6 files changed, 218 insertions(+), 192 deletions(-)

-- 
1.8.3.1

[PATCH 1/8] cifs: We do not drop reference to tlink in CIFSCheckMFSymlink()

[Sachin Prabhu]

When we obtain tcon from cifs_sb, we use cifs_sb_tlink() to first obtain
tlink which also grabs a reference to it. We do not drop this reference
to tlink once we are done with the call.

The patch fixes this issue by instead passing tcon as a parameter and
avoids having to obtain a reference to the tlink. A lookup for the tcon
is already made in the calling functions and this way we avoid having to
re-run the lookup. This is also consistent with the argument list for
other similar calls for M-F symlinks.

We should also return an ENOSYS when we do not find a protocol specific
function to lookup the MF Symlink data.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsproto.h |  7 ++++---
 fs/cifs/inode.c     |  6 ++++--
 fs/cifs/link.c      | 26 +++++++++++---------------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index aa33976..2c29db6 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -477,9 +477,10 @@ extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
 			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
 extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
 extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr);
-extern int CIFSCheckMFSymlink(struct cifs_fattr *fattr,
-		const unsigned char *path,
-		struct cifs_sb_info *cifs_sb, unsigned int xid);
+extern int CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
+			      struct cifs_sb_info *cifs_sb,
+			      struct cifs_fattr *fattr,
+			      const unsigned char *path);
 extern int mdfour(unsigned char *, unsigned char *, int);
 extern int E_md4hash(const unsigned char *passwd, unsigned char *p16,
 			const struct nls_table *codepage);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 36f9ebb..49719b8 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -383,7 +383,8 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 
 	/* check for Minshall+French symlinks */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
-		int tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
+		int tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
+					       full_path);
 		if (tmprc)
 			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
 	}
@@ -799,7 +800,8 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
 
 	/* check for Minshall+French symlinks */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
-		tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
+		tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
+					   full_path);
 		if (tmprc)
 			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
 	}
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index cc02347..92aee08 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -354,34 +354,30 @@ open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
 
 
 int
-CIFSCheckMFSymlink(struct cifs_fattr *fattr,
-		   const unsigned char *path,
-		   struct cifs_sb_info *cifs_sb, unsigned int xid)
+CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
+		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
+		   const unsigned char *path)
 {
-	int rc = 0;
+	int rc;
 	u8 *buf = NULL;
 	unsigned int link_len = 0;
 	unsigned int bytes_read = 0;
-	struct cifs_tcon *ptcon;
 
 	if (!CIFSCouldBeMFSymlink(fattr))
 		/* it's not a symlink */
 		return 0;
 
 	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
-	if (!buf) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	if (!buf)
+		return -ENOMEM;
 
-	ptcon = tlink_tcon(cifs_sb_tlink(cifs_sb));
-	if ((ptcon->ses) && (ptcon->ses->server->ops->query_mf_symlink))
-		rc = ptcon->ses->server->ops->query_mf_symlink(path, buf,
-						 &bytes_read, cifs_sb, xid);
+	if (tcon->ses->server->ops->query_mf_symlink)
+		rc = tcon->ses->server->ops->query_mf_symlink(path, buf,
+						&bytes_read, cifs_sb, xid);
 	else
-		goto out;
+		rc = -ENOSYS;
 
-	if (rc != 0)
+	if (rc)
 		goto out;
 
 	if (bytes_read == 0) /* not a symlink */
-- 
1.8.3.1

[PATCH 2/8] cifs: Rename and cleanup open_query_close_cifs_symlink()

[Sachin Prabhu]

Rename open_query_close_cifs_symlink to cifs_query_mf_symlink() to make
the name more consistent with other protocol version specific functions.

We also pass tcon as an argument to the function. This is already
available in the calling functions and we can avoid having to make an
unnecessary lookup.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h  |  5 +++--
 fs/cifs/cifsproto.h |  7 ++++---
 fs/cifs/link.c      | 37 ++++++++++++-------------------------
 fs/cifs/smb1ops.c   |  2 +-
 4 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d9ea7ad..e844515 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -370,8 +370,9 @@ struct smb_version_operations {
 	void (*new_lease_key)(struct cifs_fid *);
 	int (*generate_signingkey)(struct cifs_ses *);
 	int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *);
-	int (*query_mf_symlink)(const unsigned char *, char *, unsigned int *,
-				struct cifs_sb_info *, unsigned int);
+	int (*query_mf_symlink)(unsigned int, struct cifs_tcon *,
+				struct cifs_sb_info *, const unsigned char *,
+				char *, unsigned int *);
 	/* if we can do cache read operations */
 	bool (*is_read_op)(__u32);
 	/* set oplock level for the inode */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 2c29db6..10b9ab1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -496,7 +496,8 @@ void cifs_writev_complete(struct work_struct *work);
 struct cifs_writedata *cifs_writedata_alloc(unsigned int nr_pages,
 						work_func_t complete);
 void cifs_writedata_release(struct kref *refcount);
-int open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
-			unsigned int *pbytes_read, struct cifs_sb_info *cifs_sb,
-			unsigned int xid);
+int cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+			  struct cifs_sb_info *cifs_sb,
+			  const unsigned char *path, char *pbuf,
+			  unsigned int *pbytes_read);
 #endif			/* _CIFSPROTO_H */
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 92aee08..2a5837a 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -305,54 +305,41 @@ CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr)
 }
 
 int
-open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
-			unsigned int *pbytes_read, struct cifs_sb_info *cifs_sb,
-			unsigned int xid)
+cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+		      struct cifs_sb_info *cifs_sb, const unsigned char *path,
+		      char *pbuf, unsigned int *pbytes_read)
 {
 	int rc;
 	int oplock = 0;
 	__u16 netfid = 0;
-	struct tcon_link *tlink;
-	struct cifs_tcon *ptcon;
 	struct cifs_io_parms io_parms;
 	int buf_type = CIFS_NO_BUFFER;
 	FILE_ALL_INFO file_info;
 
-	tlink = cifs_sb_tlink(cifs_sb);
-	if (IS_ERR(tlink))
-		return PTR_ERR(tlink);
-	ptcon = tlink_tcon(tlink);
-
-	rc = CIFSSMBOpen(xid, ptcon, path, FILE_OPEN, GENERIC_READ,
+	rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, GENERIC_READ,
 			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
 			 cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
-	if (rc != 0) {
-		cifs_put_tlink(tlink);
+	if (rc)
 		return rc;
-	}
 
-	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE)) {
-		CIFSSMBClose(xid, ptcon, netfid);
-		cifs_put_tlink(tlink);
+	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE))
 		/* it's not a symlink */
-		return rc;
-	}
+		goto out;
 
 	io_parms.netfid = netfid;
 	io_parms.pid = current->tgid;
-	io_parms.tcon = ptcon;
+	io_parms.tcon = tcon;
 	io_parms.offset = 0;
 	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
 
 	rc = CIFSSMBRead(xid, &io_parms, pbytes_read, &pbuf, &buf_type);
-	CIFSSMBClose(xid, ptcon, netfid);
-	cifs_put_tlink(tlink);
+out:
+	CIFSSMBClose(xid, tcon, netfid);
 	return rc;
 }
 
-
 int
 CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
 		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
@@ -372,8 +359,8 @@ CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
 		return -ENOMEM;
 
 	if (tcon->ses->server->ops->query_mf_symlink)
-		rc = tcon->ses->server->ops->query_mf_symlink(path, buf,
-						&bytes_read, cifs_sb, xid);
+		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
+					      cifs_sb, path, buf, &bytes_read);
 	else
 		rc = -ENOSYS;
 
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 5f5ba0d..099c276 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1009,7 +1009,7 @@ struct smb_version_operations smb1_operations = {
 	.mand_lock = cifs_mand_lock,
 	.mand_unlock_range = cifs_unlock_range,
 	.push_mand_locks = cifs_push_mandatory_locks,
-	.query_mf_symlink = open_query_close_cifs_symlink,
+	.query_mf_symlink = cifs_query_mf_symlink,
 	.is_read_op = cifs_is_read_op,
 };
 
-- 
1.8.3.1

[PATCH 3/8] cifs: Rename MF symlink function names

[Sachin Prabhu]

Clean up camel case in functionnames.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsproto.h |  4 ++--
 fs/cifs/inode.c     | 12 ++++++------
 fs/cifs/link.c      | 32 +++++++++++++++-----------------
 fs/cifs/readdir.c   |  2 +-
 4 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 10b9ab1..78bb6d6 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -476,8 +476,8 @@ extern int CIFSSMBSetPosixACL(const unsigned int xid, struct cifs_tcon *tcon,
 extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
 			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
 extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
-extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr);
-extern int CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
+extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
+extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 			      struct cifs_sb_info *cifs_sb,
 			      struct cifs_fattr *fattr,
 			      const unsigned char *path);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 49719b8..6f7f57a 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -383,10 +383,10 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 
 	/* check for Minshall+French symlinks */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
-		int tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
-					       full_path);
+		int tmprc = check_mf_symlink(xid, tcon, cifs_sb, &fattr,
+					     full_path);
 		if (tmprc)
-			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
+			cifs_dbg(FYI, "check_mf_symlink: %d\n", tmprc);
 	}
 
 	if (*pinode == NULL) {
@@ -800,10 +800,10 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
 
 	/* check for Minshall+French symlinks */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
-		tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
-					   full_path);
+		tmprc = check_mf_symlink(xid, tcon, cifs_sb, &fattr,
+					 full_path);
 		if (tmprc)
-			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
+			cifs_dbg(FYI, "check_mf_symlink: %d\n", tmprc);
 	}
 
 	if (!*inode) {
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 2a5837a..2e7bbab 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -91,10 +91,8 @@ symlink_hash_err:
 }
 
 static int
-CIFSParseMFSymlink(const u8 *buf,
-		   unsigned int buf_len,
-		   unsigned int *_link_len,
-		   char **_link_str)
+parse_mf_symlink(const u8 *buf, unsigned int buf_len, unsigned int *_link_len,
+		 char **_link_str)
 {
 	int rc;
 	unsigned int link_len;
@@ -137,7 +135,7 @@ CIFSParseMFSymlink(const u8 *buf,
 }
 
 static int
-CIFSFormatMFSymlink(u8 *buf, unsigned int buf_len, const char *link_str)
+format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
 {
 	int rc;
 	unsigned int link_len;
@@ -181,7 +179,7 @@ CIFSFormatMFSymlink(u8 *buf, unsigned int buf_len, const char *link_str)
 }
 
 static int
-CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
+create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		    const char *fromName, const char *toName,
 		    struct cifs_sb_info *cifs_sb)
 {
@@ -202,7 +200,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 	if (!buf)
 		return -ENOMEM;
 
-	rc = CIFSFormatMFSymlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
+	rc = format_mf_symlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
 	if (rc != 0) {
 		kfree(buf);
 		return rc;
@@ -238,7 +236,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 }
 
 static int
-CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
+query_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		   const unsigned char *searchName, char **symlinkinfo,
 		   const struct nls_table *nls_codepage, int remap)
 {
@@ -282,7 +280,7 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 		return rc;
 	}
 
-	rc = CIFSParseMFSymlink(buf, bytes_read, &link_len, symlinkinfo);
+	rc = parse_mf_symlink(buf, bytes_read, &link_len, symlinkinfo);
 	kfree(buf);
 	if (rc != 0)
 		return rc;
@@ -291,7 +289,7 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 }
 
 bool
-CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr)
+couldbe_mf_symlink(const struct cifs_fattr *fattr)
 {
 	if (!(fattr->cf_mode & S_IFREG))
 		/* it's not a symlink */
@@ -341,16 +339,16 @@ out:
 }
 
 int
-CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
-		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
-		   const unsigned char *path)
+check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
+		 const unsigned char *path)
 {
 	int rc;
 	u8 *buf = NULL;
 	unsigned int link_len = 0;
 	unsigned int bytes_read = 0;
 
-	if (!CIFSCouldBeMFSymlink(fattr))
+	if (!couldbe_mf_symlink(fattr))
 		/* it's not a symlink */
 		return 0;
 
@@ -370,7 +368,7 @@ CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
 	if (bytes_read == 0) /* not a symlink */
 		goto out;
 
-	rc = CIFSParseMFSymlink(buf, bytes_read, &link_len, NULL);
+	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
 	if (rc == -EINVAL) {
 		/* it's not a symlink */
 		rc = 0;
@@ -517,7 +515,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 	 * and fallback to UNIX Extensions Symlinks.
 	 */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
-		rc = CIFSQueryMFSymLink(xid, tcon, full_path, &target_path,
+		rc = query_mf_symlink(xid, tcon, full_path, &target_path,
 					cifs_sb->local_nls,
 					cifs_sb->mnt_cifs_flags &
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
@@ -574,7 +572,7 @@ cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname)
 
 	/* BB what if DFS and this volume is on different share? BB */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
-		rc = CIFSCreateMFSymLink(xid, pTcon, full_path, symname,
+		rc = create_mf_symlink(xid, pTcon, full_path, symname,
 					cifs_sb);
 	else if (pTcon->unix_ext)
 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 5940eca..b15862e 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -749,7 +749,7 @@ static int cifs_filldir(char *find_entry, struct file *file,
 	}
 
 	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) &&
-	    CIFSCouldBeMFSymlink(&fattr))
+	    couldbe_mf_symlink(&fattr))
 		/*
 		 * trying to get the type and mode can be slow,
 		 * so just call those regular files for now, and mark
-- 
1.8.3.1

[PATCH 4/8] cifs: use protocol specific call for query_mf_symlink()

[Sachin Prabhu]

We have an existing protocol specific call query_mf_symlink() created
for check_mf_symlink which can also be used for query_mf_symlink().

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/link.c | 61 +++++++++++++++++++---------------------------------------
 1 file changed, 20 insertions(+), 41 deletions(-)

diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 2e7bbab..f8aaf10 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -237,55 +237,36 @@ create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 
 static int
 query_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
-		   const unsigned char *searchName, char **symlinkinfo,
-		   const struct nls_table *nls_codepage, int remap)
+		 struct cifs_sb_info *cifs_sb, const unsigned char *path,
+		 char **symlinkinfo)
 {
 	int rc;
-	int oplock = 0;
-	__u16 netfid = 0;
-	u8 *buf;
-	char *pbuf;
-	unsigned int bytes_read = 0;
-	int buf_type = CIFS_NO_BUFFER;
+	u8 *buf = NULL;
 	unsigned int link_len = 0;
-	struct cifs_io_parms io_parms;
-	FILE_ALL_INFO file_info;
-
-	rc = CIFSSMBOpen(xid, tcon, searchName, FILE_OPEN, GENERIC_READ,
-			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
-			 nls_codepage, remap);
-	if (rc != 0)
-		return rc;
-
-	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE)) {
-		CIFSSMBClose(xid, tcon, netfid);
-		/* it's not a symlink */
-		return -EINVAL;
-	}
+	unsigned int bytes_read = 0;
 
 	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
-	pbuf = buf;
-	io_parms.netfid = netfid;
-	io_parms.pid = current->tgid;
-	io_parms.tcon = tcon;
-	io_parms.offset = 0;
-	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
 
-	rc = CIFSSMBRead(xid, &io_parms, &bytes_read, &pbuf, &buf_type);
-	CIFSSMBClose(xid, tcon, netfid);
-	if (rc != 0) {
-		kfree(buf);
-		return rc;
+	if (tcon->ses->server->ops->query_mf_symlink)
+		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
+					      cifs_sb, path, buf, &bytes_read);
+	else
+		rc = -ENOSYS;
+
+	if (rc)
+		goto out;
+
+	if (bytes_read == 0) { /* not a symlink */
+		rc = -EINVAL;
+		goto out;
 	}
 
 	rc = parse_mf_symlink(buf, bytes_read, &link_len, symlinkinfo);
+out:
 	kfree(buf);
-	if (rc != 0)
-		return rc;
-
-	return 0;
+	return rc;
 }
 
 bool
@@ -515,10 +496,8 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 	 * and fallback to UNIX Extensions Symlinks.
 	 */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
-		rc = query_mf_symlink(xid, tcon, full_path, &target_path,
-					cifs_sb->local_nls,
-					cifs_sb->mnt_cifs_flags &
-						CIFS_MOUNT_MAP_SPECIAL_CHR);
+		rc = query_mf_symlink(xid, tcon, cifs_sb, full_path,
+				      &target_path);
 
 	if ((rc != 0) && cap_unix(tcon->ses))
 		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
-- 
1.8.3.1

[PATCH 5/8] cifs: Add create MFSymlinks to protocol ops struct

[Sachin Prabhu]

Add a new protocol ops function create_mf_symlink and have
create_mf_symlink() use it.

This patchset moves the MFSymlink operations completely to the
ops structure so that we only use the right protocol versions when
querying or creating MFSymlinks.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsglob.h  |  3 ++
 fs/cifs/cifsproto.h |  4 +++
 fs/cifs/link.c      | 88 ++++++++++++++++++++++++++++-------------------------
 fs/cifs/smb1ops.c   |  1 +
 4 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index e844515..1781e89 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -373,6 +373,9 @@ struct smb_version_operations {
 	int (*query_mf_symlink)(unsigned int, struct cifs_tcon *,
 				struct cifs_sb_info *, const unsigned char *,
 				char *, unsigned int *);
+	int (*create_mf_symlink)(unsigned int, struct cifs_tcon *,
+				 struct cifs_sb_info *, const unsigned char *,
+				 char *, unsigned int *);
 	/* if we can do cache read operations */
 	bool (*is_read_op)(__u32);
 	/* set oplock level for the inode */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 78bb6d6..e88c3b1 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -500,4 +500,8 @@ int cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 			  struct cifs_sb_info *cifs_sb,
 			  const unsigned char *path, char *pbuf,
 			  unsigned int *pbytes_read);
+int cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+			   struct cifs_sb_info *cifs_sb,
+			   const unsigned char *path, char *pbuf,
+			   unsigned int *pbytes_written);
 #endif			/* _CIFSPROTO_H */
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index f8aaf10..d45d43d 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -180,59 +180,31 @@ format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
 
 static int
 create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
-		    const char *fromName, const char *toName,
-		    struct cifs_sb_info *cifs_sb)
+		  struct cifs_sb_info *cifs_sb, const char *fromName,
+		  const char *toName)
 {
 	int rc;
-	int oplock = 0;
-	int remap;
-	int create_options = CREATE_NOT_DIR;
-	__u16 netfid = 0;
 	u8 *buf;
 	unsigned int bytes_written = 0;
-	struct cifs_io_parms io_parms;
-	struct nls_table *nls_codepage;
-
-	nls_codepage = cifs_sb->local_nls;
-	remap = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR;
 
 	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
 	rc = format_mf_symlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
-	if (rc != 0) {
-		kfree(buf);
-		return rc;
-	}
-
-	if (backup_cred(cifs_sb))
-		create_options |= CREATE_OPEN_BACKUP_INTENT;
-
-	rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE,
-			 create_options, &netfid, &oplock, NULL,
-			 nls_codepage, remap);
-	if (rc != 0) {
-		kfree(buf);
-		return rc;
-	}
-
-	io_parms.netfid = netfid;
-	io_parms.pid = current->tgid;
-	io_parms.tcon = tcon;
-	io_parms.offset = 0;
-	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
+	if (rc)
+		goto out;
 
-	rc = CIFSSMBWrite(xid, &io_parms, &bytes_written, buf, NULL, 0);
-	CIFSSMBClose(xid, tcon, netfid);
-	kfree(buf);
-	if (rc != 0)
-		return rc;
+	rc = tcon->ses->server->ops->create_mf_symlink(xid, tcon, cifs_sb,
+					fromName, buf, &bytes_written);
+	if (rc)
+		goto out;
 
 	if (bytes_written != CIFS_MF_SYMLINK_FILE_SIZE)
-		return -EIO;
-
-	return 0;
+		rc = -EIO;
+out:
+	kfree(buf);
+	return rc;
 }
 
 static int
@@ -320,6 +292,39 @@ out:
 }
 
 int
+cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+		       struct cifs_sb_info *cifs_sb, const unsigned char *path,
+		       char *pbuf, unsigned int *pbytes_written)
+{
+	int rc;
+	int oplock = 0;
+	__u16 netfid = 0;
+	struct cifs_io_parms io_parms;
+	int create_options = CREATE_NOT_DIR;
+
+	if (backup_cred(cifs_sb))
+		create_options |= CREATE_OPEN_BACKUP_INTENT;
+
+	rc = CIFSSMBOpen(xid, tcon, path, FILE_CREATE, GENERIC_WRITE,
+			 create_options, &netfid, &oplock, NULL,
+			 cifs_sb->local_nls,
+			 cifs_sb->mnt_cifs_flags &
+				CIFS_MOUNT_MAP_SPECIAL_CHR);
+	if (rc)
+		return rc;
+
+	io_parms.netfid = netfid;
+	io_parms.pid = current->tgid;
+	io_parms.tcon = tcon;
+	io_parms.offset = 0;
+	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
+
+	rc = CIFSSMBWrite(xid, &io_parms, pbytes_written, pbuf, NULL, 0);
+	CIFSSMBClose(xid, tcon, netfid);
+	return rc;
+}
+
+int
 check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
 		 const unsigned char *path)
@@ -551,8 +556,7 @@ cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname)
 
 	/* BB what if DFS and this volume is on different share? BB */
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
-		rc = create_mf_symlink(xid, pTcon, full_path, symname,
-					cifs_sb);
+		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
 	else if (pTcon->unix_ext)
 		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
 					   cifs_sb->local_nls);
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 099c276..1470ec4 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1010,6 +1010,7 @@ struct smb_version_operations smb1_operations = {
 	.mand_unlock_range = cifs_unlock_range,
 	.push_mand_locks = cifs_push_mandatory_locks,
 	.query_mf_symlink = cifs_query_mf_symlink,
+	.create_mf_symlink = cifs_create_mf_symlink,
 	.is_read_op = cifs_is_read_op,
 };
 
-- 
1.8.3.1

[PATCH 6/8] cifs: Re-order M-F Symlink code

[Sachin Prabhu]

This patch makes cosmetic changes. We group similar functions together
and separate out the protocol specific functions.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/link.c | 124 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 68 insertions(+), 56 deletions(-)

diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index d45d43d..ee5ae46 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -29,6 +29,10 @@
 #include "cifs_debug.h"
 #include "cifs_fs_sb.h"
 
+/*
+ * M-F Symlink Functions - Begin
+ */
+
 #define CIFS_MF_SYMLINK_LEN_OFFSET (4+1)
 #define CIFS_MF_SYMLINK_MD5_OFFSET (CIFS_MF_SYMLINK_LEN_OFFSET+(4+1))
 #define CIFS_MF_SYMLINK_LINK_OFFSET (CIFS_MF_SYMLINK_MD5_OFFSET+(32+1))
@@ -178,6 +182,20 @@ format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
 	return 0;
 }
 
+bool
+couldbe_mf_symlink(const struct cifs_fattr *fattr)
+{
+	if (!(fattr->cf_mode & S_IFREG))
+		/* it's not a symlink */
+		return false;
+
+	if (fattr->cf_eof != CIFS_MF_SYMLINK_FILE_SIZE)
+		/* it's not a symlink */
+		return false;
+
+	return true;
+}
+
 static int
 create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		  struct cifs_sb_info *cifs_sb, const char *fromName,
@@ -241,20 +259,60 @@ out:
 	return rc;
 }
 
-bool
-couldbe_mf_symlink(const struct cifs_fattr *fattr)
+int
+check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
+		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
+		 const unsigned char *path)
 {
-	if (!(fattr->cf_mode & S_IFREG))
+	int rc;
+	u8 *buf = NULL;
+	unsigned int link_len = 0;
+	unsigned int bytes_read = 0;
+
+	if (!couldbe_mf_symlink(fattr))
 		/* it's not a symlink */
-		return false;
+		return 0;
 
-	if (fattr->cf_eof != CIFS_MF_SYMLINK_FILE_SIZE)
+	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	if (tcon->ses->server->ops->query_mf_symlink)
+		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
+					      cifs_sb, path, buf, &bytes_read);
+	else
+		rc = -ENOSYS;
+
+	if (rc)
+		goto out;
+
+	if (bytes_read == 0) /* not a symlink */
+		goto out;
+
+	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
+	if (rc == -EINVAL) {
 		/* it's not a symlink */
-		return false;
+		rc = 0;
+		goto out;
+	}
 
-	return true;
+	if (rc != 0)
+		goto out;
+
+	/* it is a symlink */
+	fattr->cf_eof = link_len;
+	fattr->cf_mode &= ~S_IFMT;
+	fattr->cf_mode |= S_IFLNK | S_IRWXU | S_IRWXG | S_IRWXO;
+	fattr->cf_dtype = DT_LNK;
+out:
+	kfree(buf);
+	return rc;
 }
 
+/*
+ * SMB 1.0 Protocol specific functions
+ */
+
 int
 cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 		      struct cifs_sb_info *cifs_sb, const unsigned char *path,
@@ -324,55 +382,9 @@ cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 	return rc;
 }
 
-int
-check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
-		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
-		 const unsigned char *path)
-{
-	int rc;
-	u8 *buf = NULL;
-	unsigned int link_len = 0;
-	unsigned int bytes_read = 0;
-
-	if (!couldbe_mf_symlink(fattr))
-		/* it's not a symlink */
-		return 0;
-
-	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	if (tcon->ses->server->ops->query_mf_symlink)
-		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
-					      cifs_sb, path, buf, &bytes_read);
-	else
-		rc = -ENOSYS;
-
-	if (rc)
-		goto out;
-
-	if (bytes_read == 0) /* not a symlink */
-		goto out;
-
-	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
-	if (rc == -EINVAL) {
-		/* it's not a symlink */
-		rc = 0;
-		goto out;
-	}
-
-	if (rc != 0)
-		goto out;
-
-	/* it is a symlink */
-	fattr->cf_eof = link_len;
-	fattr->cf_mode &= ~S_IFMT;
-	fattr->cf_mode |= S_IFLNK | S_IRWXU | S_IRWXG | S_IRWXO;
-	fattr->cf_dtype = DT_LNK;
-out:
-	kfree(buf);
-	return rc;
-}
+/*
+ * M-F Symlink Functions - End
+ */
 
 int
 cifs_hardlink(struct dentry *old_file, struct inode *inode,
-- 
1.8.3.1

[PATCH 7/8] cifs: move unix extension call to cifs_query_symlink()

[Sachin Prabhu]

Unix extensions rigth now are only applicable to smb1 operations.
Move the check and subsequent unix extension call to the smb1
specific call to query_symlink() ie. cifs_query_symlink().

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/link.c    |  5 +----
 fs/cifs/smb1ops.c | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index ee5ae46..67d9243 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -516,10 +516,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 		rc = query_mf_symlink(xid, tcon, cifs_sb, full_path,
 				      &target_path);
 
-	if ((rc != 0) && cap_unix(tcon->ses))
-		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
-					     cifs_sb->local_nls);
-	else if (rc != 0 && server->ops->query_symlink)
+	if (rc != 0 && server->ops->query_symlink)
 		rc = server->ops->query_symlink(xid, tcon, full_path,
 						&target_path, cifs_sb);
 
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 1470ec4..2d822dd 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -918,23 +918,31 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 
 	cifs_dbg(FYI, "%s: path: %s\n", __func__, full_path);
 
+	/* Check for unix extensions */
+	if (cap_unix(tcon->ses)) {
+		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
+					     cifs_sb->local_nls);
+		goto out;
+	}
+
 	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
 			 FILE_READ_ATTRIBUTES, OPEN_REPARSE_POINT, &netfid,
 			 &oplock, NULL, cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc)
-		return rc;
+		goto out;
 
 	rc = CIFSSMBQuerySymLink(xid, tcon, netfid, target_path,
 				 cifs_sb->local_nls);
-	if (rc) {
-		CIFSSMBClose(xid, tcon, netfid);
-		return rc;
-	}
+	if (rc)
+		goto out_close;
 
 	convert_delimiter(*target_path, '/');
+out_close:
 	CIFSSMBClose(xid, tcon, netfid);
-	cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
+out:
+	if (!rc)
+		cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
 	return rc;
 }
 
-- 
1.8.3.1

[PATCH 8/8] cifs: Add support for readlink on dfs shares under posix extensions

[Sachin Prabhu]

When using posix extensions, dfs shares in the dfs root show up as
symlinks resulting in userland tools such as 'ls' calling readlink() on
these shares. Since these are dfs shares, readlink fails with -EREMOTE.

With added support for dfs shares on readlink when using unix
extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
return the first node returned.

The dfs share in the dfs root is now displayed in the following manner.
$ ls -l /mnt
total 0
lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test

Red Hat BZ: 1020715

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/smb1ops.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 2d822dd..abd2cc9 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
 }
 
 static int
+cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
+		       const unsigned char *searchName, char **symlinkinfo,
+		       const struct nls_table *nls_codepage)
+{
+#ifdef CONFIG_CIFS_DFS_UPCALL
+	int rc;
+	unsigned int num_referrals = 0;
+	struct dfs_info3_param *referrals = NULL;
+
+	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
+			  &num_referrals, &referrals, 0);
+
+	if (!rc && num_referrals > 0) {
+		*symlinkinfo = kstrndup(referrals->node_name,
+					strlen(referrals->node_name),
+					GFP_KERNEL);
+		if (!*symlinkinfo)
+			rc = -ENOMEM;
+		free_dfs_info_array(referrals, num_referrals);
+	}
+	return rc;
+#else /* No DFS support */
+	return -EREMOTE;
+#endif
+}
+
+static int
 cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		   const char *full_path, char **target_path,
 		   struct cifs_sb_info *cifs_sb)
@@ -920,8 +947,13 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 
 	/* Check for unix extensions */
 	if (cap_unix(tcon->ses)) {
-		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
+		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
 					     cifs_sb->local_nls);
+		if (rc == -EREMOTE)
+			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
+						    target_path,
+						    cifs_sb->local_nls);
+
 		goto out;
 	}
 
-- 
1.8.3.1

Re: [PATCH 1/8] cifs: We do not drop reference to tlink in CIFSCheckMFSymlink()

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:48 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> When we obtain tcon from cifs_sb, we use cifs_sb_tlink() to first obtain
> tlink which also grabs a reference to it. We do not drop this reference
> to tlink once we are done with the call.
> 
> The patch fixes this issue by instead passing tcon as a parameter and
> avoids having to obtain a reference to the tlink. A lookup for the tcon
> is already made in the calling functions and this way we avoid having to
> re-run the lookup. This is also consistent with the argument list for
> other similar calls for M-F symlinks.
> 
> We should also return an ENOSYS when we do not find a protocol specific
> function to lookup the MF Symlink data.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsproto.h |  7 ++++---
>  fs/cifs/inode.c     |  6 ++++--
>  fs/cifs/link.c      | 26 +++++++++++---------------
>  3 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index aa33976..2c29db6 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -477,9 +477,10 @@ extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
>  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
>  extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
>  extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr);
> -extern int CIFSCheckMFSymlink(struct cifs_fattr *fattr,
> -		const unsigned char *path,
> -		struct cifs_sb_info *cifs_sb, unsigned int xid);
> +extern int CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
> +			      struct cifs_sb_info *cifs_sb,
> +			      struct cifs_fattr *fattr,
> +			      const unsigned char *path);
>  extern int mdfour(unsigned char *, unsigned char *, int);
>  extern int E_md4hash(const unsigned char *passwd, unsigned char *p16,
>  			const struct nls_table *codepage);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 36f9ebb..49719b8 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -383,7 +383,8 @@ int cifs_get_inode_info_unix(struct inode **pinode,
>  
>  	/* check for Minshall+French symlinks */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> -		int tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
> +		int tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
> +					       full_path);
>  		if (tmprc)
>  			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
>  	}
> @@ -799,7 +800,8 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>  
>  	/* check for Minshall+French symlinks */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> -		tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
> +		tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
> +					   full_path);
>  		if (tmprc)
>  			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
>  	}
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index cc02347..92aee08 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -354,34 +354,30 @@ open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
>  
>  
>  int
> -CIFSCheckMFSymlink(struct cifs_fattr *fattr,
> -		   const unsigned char *path,
> -		   struct cifs_sb_info *cifs_sb, unsigned int xid)
> +CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
> +		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> +		   const unsigned char *path)
>  {
> -	int rc = 0;
> +	int rc;
>  	u8 *buf = NULL;
>  	unsigned int link_len = 0;
>  	unsigned int bytes_read = 0;
> -	struct cifs_tcon *ptcon;
>  
>  	if (!CIFSCouldBeMFSymlink(fattr))
>  		/* it's not a symlink */
>  		return 0;
>  
>  	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
> -	if (!buf) {
> -		rc = -ENOMEM;
> -		goto out;
> -	}
> +	if (!buf)
> +		return -ENOMEM;
>  
> -	ptcon = tlink_tcon(cifs_sb_tlink(cifs_sb));
> -	if ((ptcon->ses) && (ptcon->ses->server->ops->query_mf_symlink))
> -		rc = ptcon->ses->server->ops->query_mf_symlink(path, buf,
> -						 &bytes_read, cifs_sb, xid);
> +	if (tcon->ses->server->ops->query_mf_symlink)
> +		rc = tcon->ses->server->ops->query_mf_symlink(path, buf,
> +						&bytes_read, cifs_sb, xid);
>  	else
> -		goto out;
> +		rc = -ENOSYS;
>  
> -	if (rc != 0)
> +	if (rc)
>  		goto out;
>  
>  	if (bytes_read == 0) /* not a symlink */

This should probably also go to stable?

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 2/8] cifs: Rename and cleanup open_query_close_cifs_symlink()

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:49 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Rename open_query_close_cifs_symlink to cifs_query_mf_symlink() to make
> the name more consistent with other protocol version specific functions.
> 
> We also pass tcon as an argument to the function. This is already
> available in the calling functions and we can avoid having to make an
> unnecessary lookup.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |  5 +++--
>  fs/cifs/cifsproto.h |  7 ++++---
>  fs/cifs/link.c      | 37 ++++++++++++-------------------------
>  fs/cifs/smb1ops.c   |  2 +-
>  4 files changed, 20 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index d9ea7ad..e844515 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -370,8 +370,9 @@ struct smb_version_operations {
>  	void (*new_lease_key)(struct cifs_fid *);
>  	int (*generate_signingkey)(struct cifs_ses *);
>  	int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *);
> -	int (*query_mf_symlink)(const unsigned char *, char *, unsigned int *,
> -				struct cifs_sb_info *, unsigned int);
> +	int (*query_mf_symlink)(unsigned int, struct cifs_tcon *,
> +				struct cifs_sb_info *, const unsigned char *,
> +				char *, unsigned int *);
>  	/* if we can do cache read operations */
>  	bool (*is_read_op)(__u32);
>  	/* set oplock level for the inode */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 2c29db6..10b9ab1 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -496,7 +496,8 @@ void cifs_writev_complete(struct work_struct *work);
>  struct cifs_writedata *cifs_writedata_alloc(unsigned int nr_pages,
>  						work_func_t complete);
>  void cifs_writedata_release(struct kref *refcount);
> -int open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
> -			unsigned int *pbytes_read, struct cifs_sb_info *cifs_sb,
> -			unsigned int xid);
> +int cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +			  struct cifs_sb_info *cifs_sb,
> +			  const unsigned char *path, char *pbuf,
> +			  unsigned int *pbytes_read);
>  #endif			/* _CIFSPROTO_H */
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index 92aee08..2a5837a 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -305,54 +305,41 @@ CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr)
>  }
>  
>  int
> -open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
> -			unsigned int *pbytes_read, struct cifs_sb_info *cifs_sb,
> -			unsigned int xid)
> +cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +		      struct cifs_sb_info *cifs_sb, const unsigned char *path,
> +		      char *pbuf, unsigned int *pbytes_read)
>  {
>  	int rc;
>  	int oplock = 0;
>  	__u16 netfid = 0;
> -	struct tcon_link *tlink;
> -	struct cifs_tcon *ptcon;
>  	struct cifs_io_parms io_parms;
>  	int buf_type = CIFS_NO_BUFFER;
>  	FILE_ALL_INFO file_info;
>  
> -	tlink = cifs_sb_tlink(cifs_sb);
> -	if (IS_ERR(tlink))
> -		return PTR_ERR(tlink);
> -	ptcon = tlink_tcon(tlink);
> -
> -	rc = CIFSSMBOpen(xid, ptcon, path, FILE_OPEN, GENERIC_READ,
> +	rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, GENERIC_READ,
>  			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
>  			 cifs_sb->local_nls,
>  			 cifs_sb->mnt_cifs_flags &
>  				CIFS_MOUNT_MAP_SPECIAL_CHR);
> -	if (rc != 0) {
> -		cifs_put_tlink(tlink);
> +	if (rc)
>  		return rc;
> -	}
>  
> -	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE)) {
> -		CIFSSMBClose(xid, ptcon, netfid);
> -		cifs_put_tlink(tlink);
> +	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE))
>  		/* it's not a symlink */
> -		return rc;
> -	}
> +		goto out;
>  
>  	io_parms.netfid = netfid;
>  	io_parms.pid = current->tgid;
> -	io_parms.tcon = ptcon;
> +	io_parms.tcon = tcon;
>  	io_parms.offset = 0;
>  	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
>  
>  	rc = CIFSSMBRead(xid, &io_parms, pbytes_read, &pbuf, &buf_type);
> -	CIFSSMBClose(xid, ptcon, netfid);
> -	cifs_put_tlink(tlink);
> +out:
> +	CIFSSMBClose(xid, tcon, netfid);
>  	return rc;
>  }
>  
> -
>  int
>  CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>  		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> @@ -372,8 +359,8 @@ CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>  		return -ENOMEM;
>  
>  	if (tcon->ses->server->ops->query_mf_symlink)
> -		rc = tcon->ses->server->ops->query_mf_symlink(path, buf,
> -						&bytes_read, cifs_sb, xid);
> +		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
> +					      cifs_sb, path, buf, &bytes_read);
>  	else
>  		rc = -ENOSYS;
>  
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 5f5ba0d..099c276 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -1009,7 +1009,7 @@ struct smb_version_operations smb1_operations = {
>  	.mand_lock = cifs_mand_lock,
>  	.mand_unlock_range = cifs_unlock_range,
>  	.push_mand_locks = cifs_push_mandatory_locks,
> -	.query_mf_symlink = open_query_close_cifs_symlink,
> +	.query_mf_symlink = cifs_query_mf_symlink,
>  	.is_read_op = cifs_is_read_op,
>  };
>  

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 3/8] cifs: Rename MF symlink function names

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:50 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Clean up camel case in functionnames.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsproto.h |  4 ++--
>  fs/cifs/inode.c     | 12 ++++++------
>  fs/cifs/link.c      | 32 +++++++++++++++-----------------
>  fs/cifs/readdir.c   |  2 +-
>  4 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 10b9ab1..78bb6d6 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -476,8 +476,8 @@ extern int CIFSSMBSetPosixACL(const unsigned int xid, struct cifs_tcon *tcon,
>  extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
>  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
>  extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
> -extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr);
> -extern int CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
> +extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
> +extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  			      struct cifs_sb_info *cifs_sb,
>  			      struct cifs_fattr *fattr,
>  			      const unsigned char *path);
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 49719b8..6f7f57a 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -383,10 +383,10 @@ int cifs_get_inode_info_unix(struct inode **pinode,
>  
>  	/* check for Minshall+French symlinks */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> -		int tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
> -					       full_path);
> +		int tmprc = check_mf_symlink(xid, tcon, cifs_sb, &fattr,
> +					     full_path);
>  		if (tmprc)
> -			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
> +			cifs_dbg(FYI, "check_mf_symlink: %d\n", tmprc);
>  	}
>  
>  	if (*pinode == NULL) {
> @@ -800,10 +800,10 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>  
>  	/* check for Minshall+French symlinks */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
> -		tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
> -					   full_path);
> +		tmprc = check_mf_symlink(xid, tcon, cifs_sb, &fattr,
> +					 full_path);
>  		if (tmprc)
> -			cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
> +			cifs_dbg(FYI, "check_mf_symlink: %d\n", tmprc);
>  	}
>  
>  	if (!*inode) {
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index 2a5837a..2e7bbab 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -91,10 +91,8 @@ symlink_hash_err:
>  }
>  
>  static int
> -CIFSParseMFSymlink(const u8 *buf,
> -		   unsigned int buf_len,
> -		   unsigned int *_link_len,
> -		   char **_link_str)
> +parse_mf_symlink(const u8 *buf, unsigned int buf_len, unsigned int *_link_len,
> +		 char **_link_str)
>  {
>  	int rc;
>  	unsigned int link_len;
> @@ -137,7 +135,7 @@ CIFSParseMFSymlink(const u8 *buf,
>  }
>  
>  static int
> -CIFSFormatMFSymlink(u8 *buf, unsigned int buf_len, const char *link_str)
> +format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
>  {
>  	int rc;
>  	unsigned int link_len;
> @@ -181,7 +179,7 @@ CIFSFormatMFSymlink(u8 *buf, unsigned int buf_len, const char *link_str)
>  }
>  
>  static int
> -CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
> +create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  		    const char *fromName, const char *toName,
>  		    struct cifs_sb_info *cifs_sb)
>  {
> @@ -202,7 +200,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
>  	if (!buf)
>  		return -ENOMEM;
>  
> -	rc = CIFSFormatMFSymlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
> +	rc = format_mf_symlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
>  	if (rc != 0) {
>  		kfree(buf);
>  		return rc;
> @@ -238,7 +236,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
>  }
>  
>  static int
> -CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
> +query_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  		   const unsigned char *searchName, char **symlinkinfo,
>  		   const struct nls_table *nls_codepage, int remap)
>  {
> @@ -282,7 +280,7 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
>  		return rc;
>  	}
>  
> -	rc = CIFSParseMFSymlink(buf, bytes_read, &link_len, symlinkinfo);
> +	rc = parse_mf_symlink(buf, bytes_read, &link_len, symlinkinfo);
>  	kfree(buf);
>  	if (rc != 0)
>  		return rc;
> @@ -291,7 +289,7 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
>  }
>  
>  bool
> -CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr)
> +couldbe_mf_symlink(const struct cifs_fattr *fattr)
>  {
>  	if (!(fattr->cf_mode & S_IFREG))
>  		/* it's not a symlink */
> @@ -341,16 +339,16 @@ out:
>  }
>  
>  int
> -CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
> -		   struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> -		   const unsigned char *path)
> +check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> +		 const unsigned char *path)
>  {
>  	int rc;
>  	u8 *buf = NULL;
>  	unsigned int link_len = 0;
>  	unsigned int bytes_read = 0;
>  
> -	if (!CIFSCouldBeMFSymlink(fattr))
> +	if (!couldbe_mf_symlink(fattr))
>  		/* it's not a symlink */
>  		return 0;
>  
> @@ -370,7 +368,7 @@ CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>  	if (bytes_read == 0) /* not a symlink */
>  		goto out;
>  
> -	rc = CIFSParseMFSymlink(buf, bytes_read, &link_len, NULL);
> +	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
>  	if (rc == -EINVAL) {
>  		/* it's not a symlink */
>  		rc = 0;
> @@ -517,7 +515,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
>  	 * and fallback to UNIX Extensions Symlinks.
>  	 */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
> -		rc = CIFSQueryMFSymLink(xid, tcon, full_path, &target_path,
> +		rc = query_mf_symlink(xid, tcon, full_path, &target_path,
>  					cifs_sb->local_nls,
>  					cifs_sb->mnt_cifs_flags &
>  						CIFS_MOUNT_MAP_SPECIAL_CHR);
> @@ -574,7 +572,7 @@ cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname)
>  
>  	/* BB what if DFS and this volume is on different share? BB */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
> -		rc = CIFSCreateMFSymLink(xid, pTcon, full_path, symname,
> +		rc = create_mf_symlink(xid, pTcon, full_path, symname,
>  					cifs_sb);
>  	else if (pTcon->unix_ext)
>  		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> index 5940eca..b15862e 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -749,7 +749,7 @@ static int cifs_filldir(char *find_entry, struct file *file,
>  	}
>  
>  	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) &&
> -	    CIFSCouldBeMFSymlink(&fattr))
> +	    couldbe_mf_symlink(&fattr))
>  		/*
>  		 * trying to get the type and mode can be slow,
>  		 * so just call those regular files for now, and mark

Meh...ok...

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 4/8] cifs: use protocol specific call for query_mf_symlink()

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:51 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> We have an existing protocol specific call query_mf_symlink() created
> for check_mf_symlink which can also be used for query_mf_symlink().
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/link.c | 61 +++++++++++++++++++---------------------------------------
>  1 file changed, 20 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index 2e7bbab..f8aaf10 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -237,55 +237,36 @@ create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  
>  static int
>  query_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> -		   const unsigned char *searchName, char **symlinkinfo,
> -		   const struct nls_table *nls_codepage, int remap)
> +		 struct cifs_sb_info *cifs_sb, const unsigned char *path,
> +		 char **symlinkinfo)
>  {
>  	int rc;
> -	int oplock = 0;
> -	__u16 netfid = 0;
> -	u8 *buf;
> -	char *pbuf;
> -	unsigned int bytes_read = 0;
> -	int buf_type = CIFS_NO_BUFFER;
> +	u8 *buf = NULL;
>  	unsigned int link_len = 0;
> -	struct cifs_io_parms io_parms;
> -	FILE_ALL_INFO file_info;
> -
> -	rc = CIFSSMBOpen(xid, tcon, searchName, FILE_OPEN, GENERIC_READ,
> -			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
> -			 nls_codepage, remap);
> -	if (rc != 0)
> -		return rc;
> -
> -	if (file_info.EndOfFile != cpu_to_le64(CIFS_MF_SYMLINK_FILE_SIZE)) {
> -		CIFSSMBClose(xid, tcon, netfid);
> -		/* it's not a symlink */
> -		return -EINVAL;
> -	}
> +	unsigned int bytes_read = 0;
>  
>  	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
> -	pbuf = buf;
> -	io_parms.netfid = netfid;
> -	io_parms.pid = current->tgid;
> -	io_parms.tcon = tcon;
> -	io_parms.offset = 0;
> -	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
>  
> -	rc = CIFSSMBRead(xid, &io_parms, &bytes_read, &pbuf, &buf_type);
> -	CIFSSMBClose(xid, tcon, netfid);
> -	if (rc != 0) {
> -		kfree(buf);
> -		return rc;
> +	if (tcon->ses->server->ops->query_mf_symlink)
> +		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
> +					      cifs_sb, path, buf, &bytes_read);
> +	else
> +		rc = -ENOSYS;
> +
> +	if (rc)
> +		goto out;
> +
> +	if (bytes_read == 0) { /* not a symlink */
> +		rc = -EINVAL;
> +		goto out;
>  	}
>  
>  	rc = parse_mf_symlink(buf, bytes_read, &link_len, symlinkinfo);
> +out:
>  	kfree(buf);
> -	if (rc != 0)
> -		return rc;
> -
> -	return 0;
> +	return rc;
>  }
>  
>  bool
> @@ -515,10 +496,8 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
>  	 * and fallback to UNIX Extensions Symlinks.
>  	 */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
> -		rc = query_mf_symlink(xid, tcon, full_path, &target_path,
> -					cifs_sb->local_nls,
> -					cifs_sb->mnt_cifs_flags &
> -						CIFS_MOUNT_MAP_SPECIAL_CHR);
> +		rc = query_mf_symlink(xid, tcon, cifs_sb, full_path,
> +				      &target_path);
>  
>  	if ((rc != 0) && cap_unix(tcon->ses))
>  		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,

Nice cleanup.

Reviwed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 5/8] cifs: Add create MFSymlinks to protocol ops struct

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:52 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Add a new protocol ops function create_mf_symlink and have
> create_mf_symlink() use it.
> 
> This patchset moves the MFSymlink operations completely to the
> ops structure so that we only use the right protocol versions when
> querying or creating MFSymlinks.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsglob.h  |  3 ++
>  fs/cifs/cifsproto.h |  4 +++
>  fs/cifs/link.c      | 88 ++++++++++++++++++++++++++++-------------------------
>  fs/cifs/smb1ops.c   |  1 +
>  4 files changed, 54 insertions(+), 42 deletions(-)
> 
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index e844515..1781e89 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -373,6 +373,9 @@ struct smb_version_operations {
>  	int (*query_mf_symlink)(unsigned int, struct cifs_tcon *,
>  				struct cifs_sb_info *, const unsigned char *,
>  				char *, unsigned int *);
> +	int (*create_mf_symlink)(unsigned int, struct cifs_tcon *,
> +				 struct cifs_sb_info *, const unsigned char *,
> +				 char *, unsigned int *);
>  	/* if we can do cache read operations */
>  	bool (*is_read_op)(__u32);
>  	/* set oplock level for the inode */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 78bb6d6..e88c3b1 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -500,4 +500,8 @@ int cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  			  struct cifs_sb_info *cifs_sb,
>  			  const unsigned char *path, char *pbuf,
>  			  unsigned int *pbytes_read);
> +int cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +			   struct cifs_sb_info *cifs_sb,
> +			   const unsigned char *path, char *pbuf,
> +			   unsigned int *pbytes_written);
>  #endif			/* _CIFSPROTO_H */
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index f8aaf10..d45d43d 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -180,59 +180,31 @@ format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
>  
>  static int
>  create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> -		    const char *fromName, const char *toName,
> -		    struct cifs_sb_info *cifs_sb)
> +		  struct cifs_sb_info *cifs_sb, const char *fromName,
> +		  const char *toName)
>  {
>  	int rc;
> -	int oplock = 0;
> -	int remap;
> -	int create_options = CREATE_NOT_DIR;
> -	__u16 netfid = 0;
>  	u8 *buf;
>  	unsigned int bytes_written = 0;
> -	struct cifs_io_parms io_parms;
> -	struct nls_table *nls_codepage;
> -
> -	nls_codepage = cifs_sb->local_nls;
> -	remap = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR;
>  
>  	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>  
>  	rc = format_mf_symlink(buf, CIFS_MF_SYMLINK_FILE_SIZE, toName);
> -	if (rc != 0) {
> -		kfree(buf);
> -		return rc;
> -	}
> -
> -	if (backup_cred(cifs_sb))
> -		create_options |= CREATE_OPEN_BACKUP_INTENT;
> -
> -	rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE,
> -			 create_options, &netfid, &oplock, NULL,
> -			 nls_codepage, remap);
> -	if (rc != 0) {
> -		kfree(buf);
> -		return rc;
> -	}
> -
> -	io_parms.netfid = netfid;
> -	io_parms.pid = current->tgid;
> -	io_parms.tcon = tcon;
> -	io_parms.offset = 0;
> -	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
> +	if (rc)
> +		goto out;
>  
> -	rc = CIFSSMBWrite(xid, &io_parms, &bytes_written, buf, NULL, 0);
> -	CIFSSMBClose(xid, tcon, netfid);
> -	kfree(buf);
> -	if (rc != 0)
> -		return rc;
> +	rc = tcon->ses->server->ops->create_mf_symlink(xid, tcon, cifs_sb,
> +					fromName, buf, &bytes_written);
> +	if (rc)
> +		goto out;
>  
>  	if (bytes_written != CIFS_MF_SYMLINK_FILE_SIZE)
> -		return -EIO;
> -
> -	return 0;
> +		rc = -EIO;
> +out:
> +	kfree(buf);
> +	return rc;
>  }
>  
>  static int
> @@ -320,6 +292,39 @@ out:
>  }
>  
>  int
> +cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +		       struct cifs_sb_info *cifs_sb, const unsigned char *path,
> +		       char *pbuf, unsigned int *pbytes_written)
> +{
> +	int rc;
> +	int oplock = 0;
> +	__u16 netfid = 0;
> +	struct cifs_io_parms io_parms;
> +	int create_options = CREATE_NOT_DIR;
> +
> +	if (backup_cred(cifs_sb))
> +		create_options |= CREATE_OPEN_BACKUP_INTENT;
> +
> +	rc = CIFSSMBOpen(xid, tcon, path, FILE_CREATE, GENERIC_WRITE,
> +			 create_options, &netfid, &oplock, NULL,
> +			 cifs_sb->local_nls,
> +			 cifs_sb->mnt_cifs_flags &
> +				CIFS_MOUNT_MAP_SPECIAL_CHR);
> +	if (rc)
> +		return rc;
> +
> +	io_parms.netfid = netfid;
> +	io_parms.pid = current->tgid;
> +	io_parms.tcon = tcon;
> +	io_parms.offset = 0;
> +	io_parms.length = CIFS_MF_SYMLINK_FILE_SIZE;
> +
> +	rc = CIFSSMBWrite(xid, &io_parms, pbytes_written, pbuf, NULL, 0);
> +	CIFSSMBClose(xid, tcon, netfid);
> +	return rc;
> +}
> +
> +int
>  check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
>  		 const unsigned char *path)
> @@ -551,8 +556,7 @@ cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname)
>  
>  	/* BB what if DFS and this volume is on different share? BB */
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
> -		rc = create_mf_symlink(xid, pTcon, full_path, symname,
> -					cifs_sb);
> +		rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
>  	else if (pTcon->unix_ext)
>  		rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
>  					   cifs_sb->local_nls);
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 099c276..1470ec4 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -1010,6 +1010,7 @@ struct smb_version_operations smb1_operations = {
>  	.mand_unlock_range = cifs_unlock_range,
>  	.push_mand_locks = cifs_push_mandatory_locks,
>  	.query_mf_symlink = cifs_query_mf_symlink,
> +	.create_mf_symlink = cifs_create_mf_symlink,
>  	.is_read_op = cifs_is_read_op,
>  };
>  

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 6/8] cifs: Re-order M-F Symlink code

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:53 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> This patch makes cosmetic changes. We group similar functions together
> and separate out the protocol specific functions.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/link.c | 124 +++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 68 insertions(+), 56 deletions(-)
> 
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index d45d43d..ee5ae46 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -29,6 +29,10 @@
>  #include "cifs_debug.h"
>  #include "cifs_fs_sb.h"
>  
> +/*
> + * M-F Symlink Functions - Begin
> + */
> +
>  #define CIFS_MF_SYMLINK_LEN_OFFSET (4+1)
>  #define CIFS_MF_SYMLINK_MD5_OFFSET (CIFS_MF_SYMLINK_LEN_OFFSET+(4+1))
>  #define CIFS_MF_SYMLINK_LINK_OFFSET (CIFS_MF_SYMLINK_MD5_OFFSET+(32+1))
> @@ -178,6 +182,20 @@ format_mf_symlink(u8 *buf, unsigned int buf_len, const char *link_str)
>  	return 0;
>  }
>  
> +bool
> +couldbe_mf_symlink(const struct cifs_fattr *fattr)
> +{
> +	if (!(fattr->cf_mode & S_IFREG))
> +		/* it's not a symlink */
> +		return false;
> +
> +	if (fattr->cf_eof != CIFS_MF_SYMLINK_FILE_SIZE)
> +		/* it's not a symlink */
> +		return false;
> +
> +	return true;
> +}
> +
>  static int
>  create_mf_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  		  struct cifs_sb_info *cifs_sb, const char *fromName,
> @@ -241,20 +259,60 @@ out:
>  	return rc;
>  }
>  
> -bool
> -couldbe_mf_symlink(const struct cifs_fattr *fattr)
> +int
> +check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> +		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> +		 const unsigned char *path)
>  {
> -	if (!(fattr->cf_mode & S_IFREG))
> +	int rc;
> +	u8 *buf = NULL;
> +	unsigned int link_len = 0;
> +	unsigned int bytes_read = 0;
> +
> +	if (!couldbe_mf_symlink(fattr))
>  		/* it's not a symlink */
> -		return false;
> +		return 0;
>  
> -	if (fattr->cf_eof != CIFS_MF_SYMLINK_FILE_SIZE)
> +	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	if (tcon->ses->server->ops->query_mf_symlink)
> +		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
> +					      cifs_sb, path, buf, &bytes_read);
> +	else
> +		rc = -ENOSYS;
> +
> +	if (rc)
> +		goto out;
> +
> +	if (bytes_read == 0) /* not a symlink */
> +		goto out;
> +
> +	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
> +	if (rc == -EINVAL) {
>  		/* it's not a symlink */
> -		return false;
> +		rc = 0;
> +		goto out;
> +	}
>  
> -	return true;
> +	if (rc != 0)
> +		goto out;
> +
> +	/* it is a symlink */
> +	fattr->cf_eof = link_len;
> +	fattr->cf_mode &= ~S_IFMT;
> +	fattr->cf_mode |= S_IFLNK | S_IRWXU | S_IRWXG | S_IRWXO;
> +	fattr->cf_dtype = DT_LNK;
> +out:
> +	kfree(buf);
> +	return rc;
>  }
>  
> +/*
> + * SMB 1.0 Protocol specific functions
> + */
> +
>  int
>  cifs_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  		      struct cifs_sb_info *cifs_sb, const unsigned char *path,
> @@ -324,55 +382,9 @@ cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  	return rc;
>  }
>  
> -int
> -check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> -		 struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> -		 const unsigned char *path)
> -{
> -	int rc;
> -	u8 *buf = NULL;
> -	unsigned int link_len = 0;
> -	unsigned int bytes_read = 0;
> -
> -	if (!couldbe_mf_symlink(fattr))
> -		/* it's not a symlink */
> -		return 0;
> -
> -	buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
> -	if (!buf)
> -		return -ENOMEM;
> -
> -	if (tcon->ses->server->ops->query_mf_symlink)
> -		rc = tcon->ses->server->ops->query_mf_symlink(xid, tcon,
> -					      cifs_sb, path, buf, &bytes_read);
> -	else
> -		rc = -ENOSYS;
> -
> -	if (rc)
> -		goto out;
> -
> -	if (bytes_read == 0) /* not a symlink */
> -		goto out;
> -
> -	rc = parse_mf_symlink(buf, bytes_read, &link_len, NULL);
> -	if (rc == -EINVAL) {
> -		/* it's not a symlink */
> -		rc = 0;
> -		goto out;
> -	}
> -
> -	if (rc != 0)
> -		goto out;
> -
> -	/* it is a symlink */
> -	fattr->cf_eof = link_len;
> -	fattr->cf_mode &= ~S_IFMT;
> -	fattr->cf_mode |= S_IFLNK | S_IRWXU | S_IRWXG | S_IRWXO;
> -	fattr->cf_dtype = DT_LNK;
> -out:
> -	kfree(buf);
> -	return rc;
> -}
> +/*
> + * M-F Symlink Functions - End
> + */
>  
>  int
>  cifs_hardlink(struct dentry *old_file, struct inode *inode,

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 7/8] cifs: move unix extension call to cifs_query_symlink()

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:54 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Unix extensions rigth now are only applicable to smb1 operations.
> Move the check and subsequent unix extension call to the smb1
> specific call to query_symlink() ie. cifs_query_symlink().
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/link.c    |  5 +----
>  fs/cifs/smb1ops.c | 20 ++++++++++++++------
>  2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
> index ee5ae46..67d9243 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -516,10 +516,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
>  		rc = query_mf_symlink(xid, tcon, cifs_sb, full_path,
>  				      &target_path);
>  
> -	if ((rc != 0) && cap_unix(tcon->ses))
> -		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
> -					     cifs_sb->local_nls);
> -	else if (rc != 0 && server->ops->query_symlink)
> +	if (rc != 0 && server->ops->query_symlink)
>  		rc = server->ops->query_symlink(xid, tcon, full_path,
>  						&target_path, cifs_sb);
>  
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 1470ec4..2d822dd 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -918,23 +918,31 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  
>  	cifs_dbg(FYI, "%s: path: %s\n", __func__, full_path);
>  
> +	/* Check for unix extensions */
> +	if (cap_unix(tcon->ses)) {
> +		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
> +					     cifs_sb->local_nls);
> +		goto out;
> +	}
> +
>  	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
>  			 FILE_READ_ATTRIBUTES, OPEN_REPARSE_POINT, &netfid,
>  			 &oplock, NULL, cifs_sb->local_nls,
>  			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>  	if (rc)
> -		return rc;
> +		goto out;
>  
>  	rc = CIFSSMBQuerySymLink(xid, tcon, netfid, target_path,
>  				 cifs_sb->local_nls);
> -	if (rc) {
> -		CIFSSMBClose(xid, tcon, netfid);
> -		return rc;
> -	}
> +	if (rc)
> +		goto out_close;
>  
>  	convert_delimiter(*target_path, '/');
> +out_close:
>  	CIFSSMBClose(xid, tcon, netfid);
> -	cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
> +out:
> +	if (!rc)
> +		cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
>  	return rc;
>  }
>  

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 8/8] cifs: Add support for readlink on dfs shares under posix extensions

[Jeff Layton]

On Mon, 25 Nov 2013 17:09:55 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> When using posix extensions, dfs shares in the dfs root show up as
> symlinks resulting in userland tools such as 'ls' calling readlink() on
> these shares. Since these are dfs shares, readlink fails with -EREMOTE.
> 
> With added support for dfs shares on readlink when using unix
> extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
> return the first node returned.
> 
> The dfs share in the dfs root is now displayed in the following manner.
> $ ls -l /mnt
> total 0
> lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test
> 

nit: I know that DFS referrals are prefixed with a single backslash,
but it might look more like a UNC with a double backslash prefix:

    lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \\vm140-31\test

...but I don't feel too strongly about it.

> Red Hat BZ: 1020715
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/smb1ops.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 2d822dd..abd2cc9 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
>  }
>  
>  static int
> +cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
> +		       const unsigned char *searchName, char **symlinkinfo,
> +		       const struct nls_table *nls_codepage)
> +{
> +#ifdef CONFIG_CIFS_DFS_UPCALL
> +	int rc;
> +	unsigned int num_referrals = 0;
> +	struct dfs_info3_param *referrals = NULL;
> +
> +	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
> +			  &num_referrals, &referrals, 0);
> +
> +	if (!rc && num_referrals > 0) {
> +		*symlinkinfo = kstrndup(referrals->node_name,
> +					strlen(referrals->node_name),
> +					GFP_KERNEL);
> +		if (!*symlinkinfo)
> +			rc = -ENOMEM;
> +		free_dfs_info_array(referrals, num_referrals);
> +	}
> +	return rc;
> +#else /* No DFS support */
> +	return -EREMOTE;
> +#endif
> +}
> +
> +static int
>  cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  		   const char *full_path, char **target_path,
>  		   struct cifs_sb_info *cifs_sb)
> @@ -920,8 +947,13 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  
>  	/* Check for unix extensions */
>  	if (cap_unix(tcon->ses)) {
> -		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
> +		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
>  					     cifs_sb->local_nls);
> +		if (rc == -EREMOTE)
> +			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
> +						    target_path,
> +						    cifs_sb->local_nls);
> +
>  		goto out;
>  	}
>  


Either way, this is a marked improvement:

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 8/8] cifs: Add support for readlink on dfs shares under posix extensions

[Sachin Prabhu]

On Wed, 2013-11-27 at 06:45 -0500, Jeff Layton wrote:
> On Mon, 25 Nov 2013 17:09:55 +0000
> Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > When using posix extensions, dfs shares in the dfs root show up as
> > symlinks resulting in userland tools such as 'ls' calling readlink() on
> > these shares. Since these are dfs shares, readlink fails with -EREMOTE.
> > 
> > With added support for dfs shares on readlink when using unix
> > extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
> > return the first node returned.
> > 
> > The dfs share in the dfs root is now displayed in the following manner.
> > $ ls -l /mnt
> > total 0
> > lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test
> > 
> 
> nit: I know that DFS referrals are prefixed with a single backslash,
> but it might look more like a UNC with a double backslash prefix:
> 
>     lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \\vm140-31\test
> 
> ...but I don't feel too strongly about it.
> 
> > Red Hat BZ: 1020715
> > 
> > Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/smb1ops.c | 34 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> > index 2d822dd..abd2cc9 100644
> > --- a/fs/cifs/smb1ops.c
> > +++ b/fs/cifs/smb1ops.c
> > @@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
> >  }
> >  
> >  static int
> > +cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
> > +		       const unsigned char *searchName, char **symlinkinfo,
> > +		       const struct nls_table *nls_codepage)
> > +{
> > +#ifdef CONFIG_CIFS_DFS_UPCALL
> > +	int rc;
> > +	unsigned int num_referrals = 0;
> > +	struct dfs_info3_param *referrals = NULL;
> > +
> > +	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
> > +			  &num_referrals, &referrals, 0);
> > +
> > +	if (!rc && num_referrals > 0) {
> > +		*symlinkinfo = kstrndup(referrals->node_name,
> > +					strlen(referrals->node_name),
> > +					GFP_KERNEL);
> > +		if (!*symlinkinfo)
> > +			rc = -ENOMEM;
> > +		free_dfs_info_array(referrals, num_referrals);
> > +	}
> > +	return rc;
> > +#else /* No DFS support */
> > +	return -EREMOTE;
> > +#endif
> > +}
> > +
> > +static int
> >  cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> >  		   const char *full_path, char **target_path,
> >  		   struct cifs_sb_info *cifs_sb)
> > @@ -920,8 +947,13 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> >  
> >  	/* Check for unix extensions */
> >  	if (cap_unix(tcon->ses)) {
> > -		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
> > +		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,

It looks like I made a mistake in this line. When writing patch #7, I
introduced a bug here which results in a compilation error. I then wrote
the fix and applied it to the wrong patch ie. patch #8 instead of #7. I
will resend patches 7 and 8. 

For now, I'll just resend the patches without including the proposed
change from Jeff so that we can get this error fixed quickly. If needed,
we can change the format in which the remote node is printed in a
separate patch.

Sachin Prabhu
>>  					     cifs_sb->local_nls);
> > +		if (rc == -EREMOTE)
> > +			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
> > +						    target_path,
> > +						    cifs_sb->local_nls);
> > +
> >  		goto out;
> >  	}
> >  
> 
> 
> Either way, this is a marked improvement:
> 
> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 8/8] cifs: Add support for readlink on dfs shares under posix extensions

[Jeff Layton]

On Wed, 27 Nov 2013 12:58:51 +0000
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Wed, 2013-11-27 at 06:45 -0500, Jeff Layton wrote:
> > On Mon, 25 Nov 2013 17:09:55 +0000
> > Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > 
> > > When using posix extensions, dfs shares in the dfs root show up as
> > > symlinks resulting in userland tools such as 'ls' calling readlink() on
> > > these shares. Since these are dfs shares, readlink fails with -EREMOTE.
> > > 
> > > With added support for dfs shares on readlink when using unix
> > > extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
> > > return the first node returned.
> > > 
> > > The dfs share in the dfs root is now displayed in the following manner.
> > > $ ls -l /mnt
> > > total 0
> > > lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test
> > > 
> > 
> > nit: I know that DFS referrals are prefixed with a single backslash,
> > but it might look more like a UNC with a double backslash prefix:
> > 
> >     lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \\vm140-31\test
> > 
> > ...but I don't feel too strongly about it.
> > 
> > > Red Hat BZ: 1020715
> > > 
> > > Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  fs/cifs/smb1ops.c | 34 +++++++++++++++++++++++++++++++++-
> > >  1 file changed, 33 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> > > index 2d822dd..abd2cc9 100644
> > > --- a/fs/cifs/smb1ops.c
> > > +++ b/fs/cifs/smb1ops.c
> > > @@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
> > >  }
> > >  
> > >  static int
> > > +cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
> > > +		       const unsigned char *searchName, char **symlinkinfo,
> > > +		       const struct nls_table *nls_codepage)
> > > +{
> > > +#ifdef CONFIG_CIFS_DFS_UPCALL
> > > +	int rc;
> > > +	unsigned int num_referrals = 0;
> > > +	struct dfs_info3_param *referrals = NULL;
> > > +
> > > +	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
> > > +			  &num_referrals, &referrals, 0);
> > > +
> > > +	if (!rc && num_referrals > 0) {
> > > +		*symlinkinfo = kstrndup(referrals->node_name,
> > > +					strlen(referrals->node_name),
> > > +					GFP_KERNEL);
> > > +		if (!*symlinkinfo)
> > > +			rc = -ENOMEM;
> > > +		free_dfs_info_array(referrals, num_referrals);
> > > +	}
> > > +	return rc;
> > > +#else /* No DFS support */
> > > +	return -EREMOTE;
> > > +#endif
> > > +}
> > > +
> > > +static int
> > >  cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> > >  		   const char *full_path, char **target_path,
> > >  		   struct cifs_sb_info *cifs_sb)
> > > @@ -920,8 +947,13 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
> > >  
> > >  	/* Check for unix extensions */
> > >  	if (cap_unix(tcon->ses)) {
> > > -		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
> > > +		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
> 
> It looks like I made a mistake in this line. When writing patch #7, I
> introduced a bug here which results in a compilation error. I then wrote
> the fix and applied it to the wrong patch ie. patch #8 instead of #7. I
> will resend patches 7 and 8. 
> 
> For now, I'll just resend the patches without including the proposed
> change from Jeff so that we can get this error fixed quickly. If needed,
> we can change the format in which the remote node is printed in a
> separate patch.
> 
> Sachin Prabhu

Fair enough. In fact, might be reasonable to follow the format that
samba's DFS symlinks use. Something like this, maybe?

    lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> msdfs:\vm140-31\test

> >>  					     cifs_sb->local_nls);
> > > +		if (rc == -EREMOTE)
> > > +			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
> > > +						    target_path,
> > > +						    cifs_sb->local_nls);
> > > +
> > >  		goto out;
> > >  	}
> > >  
> > 
> > 
> > Either way, this is a marked improvement:
> > 
> > Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> 


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[PATCH 7/8 v2] cifs: move unix extension call to cifs_query_symlink()

[Sachin Prabhu]

Unix extensions rigth now are only applicable to smb1 operations.
Move the check and subsequent unix extension call to the smb1
specific call to query_symlink() ie. cifs_query_symlink().

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/link.c    |  5 +----
 fs/cifs/smb1ops.c | 20 ++++++++++++++------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index ee5ae46..67d9243 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -516,10 +516,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
 		rc = query_mf_symlink(xid, tcon, cifs_sb, full_path,
 				      &target_path);
 
-	if ((rc != 0) && cap_unix(tcon->ses))
-		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
-					     cifs_sb->local_nls);
-	else if (rc != 0 && server->ops->query_symlink)
+	if (rc != 0 && server->ops->query_symlink)
 		rc = server->ops->query_symlink(xid, tcon, full_path,
 						&target_path, cifs_sb);
 
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 1470ec4..988fddb7 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -918,23 +918,31 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 
 	cifs_dbg(FYI, "%s: path: %s\n", __func__, full_path);
 
+	/* Check for unix extensions */
+	if (cap_unix(tcon->ses)) {
+		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
+					     cifs_sb->local_nls);
+		goto out;
+	}
+
 	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
 			 FILE_READ_ATTRIBUTES, OPEN_REPARSE_POINT, &netfid,
 			 &oplock, NULL, cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc)
-		return rc;
+		goto out;
 
 	rc = CIFSSMBQuerySymLink(xid, tcon, netfid, target_path,
 				 cifs_sb->local_nls);
-	if (rc) {
-		CIFSSMBClose(xid, tcon, netfid);
-		return rc;
-	}
+	if (rc)
+		goto out_close;
 
 	convert_delimiter(*target_path, '/');
+out_close:
 	CIFSSMBClose(xid, tcon, netfid);
-	cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
+out:
+	if (!rc)
+		cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path);
 	return rc;
 }
 
-- 
1.8.3.1

[PATCH 8/8 v2] cifs: Add support for readlink on dfs shares under posix extensions

[Sachin Prabhu]

When using posix extensions, dfs shares in the dfs root show up as
symlinks resulting in userland tools such as 'ls' calling readlink() on
these shares. Since these are dfs shares, readlink fails with -EREMOTE.

With added support for dfs shares on readlink when using unix
extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
return the first node returned.

The dfs share in the dfs root is now displayed in the following manner.
$ ls -l /mnt
total 0
lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/smb1ops.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 988fddb7..abd2cc9 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
 }
 
 static int
+cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
+		       const unsigned char *searchName, char **symlinkinfo,
+		       const struct nls_table *nls_codepage)
+{
+#ifdef CONFIG_CIFS_DFS_UPCALL
+	int rc;
+	unsigned int num_referrals = 0;
+	struct dfs_info3_param *referrals = NULL;
+
+	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
+			  &num_referrals, &referrals, 0);
+
+	if (!rc && num_referrals > 0) {
+		*symlinkinfo = kstrndup(referrals->node_name,
+					strlen(referrals->node_name),
+					GFP_KERNEL);
+		if (!*symlinkinfo)
+			rc = -ENOMEM;
+		free_dfs_info_array(referrals, num_referrals);
+	}
+	return rc;
+#else /* No DFS support */
+	return -EREMOTE;
+#endif
+}
+
+static int
 cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		   const char *full_path, char **target_path,
 		   struct cifs_sb_info *cifs_sb)
@@ -922,6 +949,11 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 	if (cap_unix(tcon->ses)) {
 		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
 					     cifs_sb->local_nls);
+		if (rc == -EREMOTE)
+			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
+						    target_path,
+						    cifs_sb->local_nls);
+
 		goto out;
 	}
 
-- 
1.8.3.1

Re: [PATCH 8/8] cifs: Add support for readlink on dfs shares under posix extensions

[Christoph Hellwig]

I was going to complain that no filesystem should implement readlink
itself but use the generic wrappers around ->follow_link, but it turns
our you actually do implement follow_link through that query_link
API.

Please fix the subject, though.

[PATCH 8/8 v3] cifs: Add support for follow_link on dfs shares under posix extensions

[Sachin Prabhu]

When using posix extensions, dfs shares in the dfs root show up as
symlinks resulting in userland tools such as 'ls' calling readlink() on
these shares. Since these are dfs shares, we end up returning -EREMOTE.

$ ls -l /mnt
ls: cannot read symbolic link /mnt/test: Object is remote
total 0
lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test

With added follow_link() support for dfs shares, when using unix
extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
return the first node returned.

The dfs share in the dfs root is now displayed in the following manner.
$ ls -l /mnt
total 0
lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/smb1ops.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 988fddb7..abd2cc9 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
 }
 
 static int
+cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
+		       const unsigned char *searchName, char **symlinkinfo,
+		       const struct nls_table *nls_codepage)
+{
+#ifdef CONFIG_CIFS_DFS_UPCALL
+	int rc;
+	unsigned int num_referrals = 0;
+	struct dfs_info3_param *referrals = NULL;
+
+	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
+			  &num_referrals, &referrals, 0);
+
+	if (!rc && num_referrals > 0) {
+		*symlinkinfo = kstrndup(referrals->node_name,
+					strlen(referrals->node_name),
+					GFP_KERNEL);
+		if (!*symlinkinfo)
+			rc = -ENOMEM;
+		free_dfs_info_array(referrals, num_referrals);
+	}
+	return rc;
+#else /* No DFS support */
+	return -EREMOTE;
+#endif
+}
+
+static int
 cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 		   const char *full_path, char **target_path,
 		   struct cifs_sb_info *cifs_sb)
@@ -922,6 +949,11 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
 	if (cap_unix(tcon->ses)) {
 		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
 					     cifs_sb->local_nls);
+		if (rc == -EREMOTE)
+			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
+						    target_path,
+						    cifs_sb->local_nls);
+
 		goto out;
 	}
 
-- 
1.8.3.1

Re: [PATCH 8/8 v2] cifs: Add support for readlink on dfs shares under posix extensions

[Sachin Prabhu]

I have posted version 3 of this patch incorporating changes suggested by
Christoph. The changes made are limited to the patch description with no
changes to the code.

Sachin Prabhu

On Wed, 2013-11-27 at 13:27 +0000, Sachin Prabhu wrote:
> When using posix extensions, dfs shares in the dfs root show up as
> symlinks resulting in userland tools such as 'ls' calling readlink() on
> these shares. Since these are dfs shares, readlink fails with -EREMOTE.
> 
> With added support for dfs shares on readlink when using unix
> extensions, we call GET_DFS_REFERRAL to obtain the DFS referral and
> return the first node returned.
> 
> The dfs share in the dfs root is now displayed in the following manner.
> $ ls -l /mnt
> total 0
> lrwxrwxrwx. 1 root root 19 Nov  6 09:47 test -> \vm140-31\test
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/smb1ops.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index 988fddb7..abd2cc9 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -908,6 +908,33 @@ cifs_mand_lock(const unsigned int xid, struct cifsFileInfo *cfile, __u64 offset,
>  }
>  
>  static int
> +cifs_unix_dfs_readlink(const unsigned int xid, struct cifs_tcon *tcon,
> +		       const unsigned char *searchName, char **symlinkinfo,
> +		       const struct nls_table *nls_codepage)
> +{
> +#ifdef CONFIG_CIFS_DFS_UPCALL
> +	int rc;
> +	unsigned int num_referrals = 0;
> +	struct dfs_info3_param *referrals = NULL;
> +
> +	rc = get_dfs_path(xid, tcon->ses, searchName, nls_codepage,
> +			  &num_referrals, &referrals, 0);
> +
> +	if (!rc && num_referrals > 0) {
> +		*symlinkinfo = kstrndup(referrals->node_name,
> +					strlen(referrals->node_name),
> +					GFP_KERNEL);
> +		if (!*symlinkinfo)
> +			rc = -ENOMEM;
> +		free_dfs_info_array(referrals, num_referrals);
> +	}
> +	return rc;
> +#else /* No DFS support */
> +	return -EREMOTE;
> +#endif
> +}
> +
> +static int
>  cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  		   const char *full_path, char **target_path,
>  		   struct cifs_sb_info *cifs_sb)
> @@ -922,6 +949,11 @@ cifs_query_symlink(const unsigned int xid, struct cifs_tcon *tcon,
>  	if (cap_unix(tcon->ses)) {
>  		rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, target_path,
>  					     cifs_sb->local_nls);
> +		if (rc == -EREMOTE)
> +			rc = cifs_unix_dfs_readlink(xid, tcon, full_path,
> +						    target_path,
> +						    cifs_sb->local_nls);
> +
>  		goto out;
>  	}
>  

Re: [PATCH 1/8] cifs: We do not drop reference to tlink in CIFSCheckMFSymlink()

[Steve French]

This looks like the only one small enough to merge for 3.13 (and I put
in cifs-2.6.git for-next).  The others in the series for 3.14.
Opinions?

On Wed, Nov 27, 2013 at 5:35 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 25 Nov 2013 17:09:48 +0000
> Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>> When we obtain tcon from cifs_sb, we use cifs_sb_tlink() to first obtain
>> tlink which also grabs a reference to it. We do not drop this reference
>> to tlink once we are done with the call.
>>
>> The patch fixes this issue by instead passing tcon as a parameter and
>> avoids having to obtain a reference to the tlink. A lookup for the tcon
>> is already made in the calling functions and this way we avoid having to
>> re-run the lookup. This is also consistent with the argument list for
>> other similar calls for M-F symlinks.
>>
>> We should also return an ENOSYS when we do not find a protocol specific
>> function to lookup the MF Symlink data.
>>
>> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  fs/cifs/cifsproto.h |  7 ++++---
>>  fs/cifs/inode.c     |  6 ++++--
>>  fs/cifs/link.c      | 26 +++++++++++---------------
>>  3 files changed, 19 insertions(+), 20 deletions(-)
>>
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index aa33976..2c29db6 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -477,9 +477,10 @@ extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
>>                       const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
>>  extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
>>  extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr *fattr);
>> -extern int CIFSCheckMFSymlink(struct cifs_fattr *fattr,
>> -             const unsigned char *path,
>> -             struct cifs_sb_info *cifs_sb, unsigned int xid);
>> +extern int CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>> +                           struct cifs_sb_info *cifs_sb,
>> +                           struct cifs_fattr *fattr,
>> +                           const unsigned char *path);
>>  extern int mdfour(unsigned char *, unsigned char *, int);
>>  extern int E_md4hash(const unsigned char *passwd, unsigned char *p16,
>>                       const struct nls_table *codepage);
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index 36f9ebb..49719b8 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -383,7 +383,8 @@ int cifs_get_inode_info_unix(struct inode **pinode,
>>
>>       /* check for Minshall+French symlinks */
>>       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
>> -             int tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
>> +             int tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
>> +                                            full_path);
>>               if (tmprc)
>>                       cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
>>       }
>> @@ -799,7 +800,8 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>>
>>       /* check for Minshall+French symlinks */
>>       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
>> -             tmprc = CIFSCheckMFSymlink(&fattr, full_path, cifs_sb, xid);
>> +             tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb, &fattr,
>> +                                        full_path);
>>               if (tmprc)
>>                       cifs_dbg(FYI, "CIFSCheckMFSymlink: %d\n", tmprc);
>>       }
>> diff --git a/fs/cifs/link.c b/fs/cifs/link.c
>> index cc02347..92aee08 100644
>> --- a/fs/cifs/link.c
>> +++ b/fs/cifs/link.c
>> @@ -354,34 +354,30 @@ open_query_close_cifs_symlink(const unsigned char *path, char *pbuf,
>>
>>
>>  int
>> -CIFSCheckMFSymlink(struct cifs_fattr *fattr,
>> -                const unsigned char *path,
>> -                struct cifs_sb_info *cifs_sb, unsigned int xid)
>> +CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>> +                struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
>> +                const unsigned char *path)
>>  {
>> -     int rc = 0;
>> +     int rc;
>>       u8 *buf = NULL;
>>       unsigned int link_len = 0;
>>       unsigned int bytes_read = 0;
>> -     struct cifs_tcon *ptcon;
>>
>>       if (!CIFSCouldBeMFSymlink(fattr))
>>               /* it's not a symlink */
>>               return 0;
>>
>>       buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
>> -     if (!buf) {
>> -             rc = -ENOMEM;
>> -             goto out;
>> -     }
>> +     if (!buf)
>> +             return -ENOMEM;
>>
>> -     ptcon = tlink_tcon(cifs_sb_tlink(cifs_sb));
>> -     if ((ptcon->ses) && (ptcon->ses->server->ops->query_mf_symlink))
>> -             rc = ptcon->ses->server->ops->query_mf_symlink(path, buf,
>> -                                              &bytes_read, cifs_sb, xid);
>> +     if (tcon->ses->server->ops->query_mf_symlink)
>> +             rc = tcon->ses->server->ops->query_mf_symlink(path, buf,
>> +                                             &bytes_read, cifs_sb, xid);
>>       else
>> -             goto out;
>> +             rc = -ENOSYS;
>>
>> -     if (rc != 0)
>> +     if (rc)
>>               goto out;
>>
>>       if (bytes_read == 0) /* not a symlink */
>
> This should probably also go to stable?
>
> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> --
> 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



-- 
Thanks,

Steve

Re: [PATCH 1/8] cifs: We do not drop reference to tlink in CIFSCheckMFSymlink()

[Sachin Prabhu]

On Mon, 2013-12-09 at 09:37 -0600, Steve French wrote:
> This looks like the only one small enough to merge for 3.13 (and I put
> in cifs-2.6.git for-next).  The others in the series for 3.14.
>  Opinions?
> 

That's fine. The other code is mostly code cleanup which can be fixed to
the next version and a bug which isn't very high on the priority list.

Sachin Prabhu

> 
> On Mon, Nov 25, 2013 at 11:09 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> wrote:
>         When we obtain tcon from cifs_sb, we use cifs_sb_tlink() to
>         first obtain
>         tlink which also grabs a reference to it. We do not drop this
>         reference
>         to tlink once we are done with the call.
>         
>         The patch fixes this issue by instead passing tcon as a
>         parameter and
>         avoids having to obtain a reference to the tlink. A lookup for
>         the tcon
>         is already made in the calling functions and this way we avoid
>         having to
>         re-run the lookup. This is also consistent with the argument
>         list for
>         other similar calls for M-F symlinks.
>         
>         We should also return an ENOSYS when we do not find a protocol
>         specific
>         function to lookup the MF Symlink data.
>         
>         Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>         ---
>          fs/cifs/cifsproto.h |  7 ++++---
>          fs/cifs/inode.c     |  6 ++++--
>          fs/cifs/link.c      | 26 +++++++++++---------------
>          3 files changed, 19 insertions(+), 20 deletions(-)
>         
>         diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>         index aa33976..2c29db6 100644
>         --- a/fs/cifs/cifsproto.h
>         +++ b/fs/cifs/cifsproto.h
>         @@ -477,9 +477,10 @@ extern int CIFSGetExtAttr(const unsigned
>         int xid, struct cifs_tcon *tcon,
>                                 const int netfid, __u64 *pExtAttrBits,
>         __u64 *pMask);
>          extern void cifs_autodisable_serverino(struct cifs_sb_info
>         *cifs_sb);
>          extern bool CIFSCouldBeMFSymlink(const struct cifs_fattr
>         *fattr);
>         -extern int CIFSCheckMFSymlink(struct cifs_fattr *fattr,
>         -               const unsigned char *path,
>         -               struct cifs_sb_info *cifs_sb, unsigned int
>         xid);
>         +extern int CIFSCheckMFSymlink(unsigned int xid, struct
>         cifs_tcon *tcon,
>         +                             struct cifs_sb_info *cifs_sb,
>         +                             struct cifs_fattr *fattr,
>         +                             const unsigned char *path);
>          extern int mdfour(unsigned char *, unsigned char *, int);
>          extern int E_md4hash(const unsigned char *passwd, unsigned
>         char *p16,
>                                 const struct nls_table *codepage);
>         diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>         index 36f9ebb..49719b8 100644
>         --- a/fs/cifs/inode.c
>         +++ b/fs/cifs/inode.c
>         @@ -383,7 +383,8 @@ int cifs_get_inode_info_unix(struct inode
>         **pinode,
>         
>                 /* check for Minshall+French symlinks */
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
>         {
>         -               int tmprc = CIFSCheckMFSymlink(&fattr,
>         full_path, cifs_sb, xid);
>         +               int tmprc = CIFSCheckMFSymlink(xid, tcon,
>         cifs_sb, &fattr,
>         +                                              full_path);
>                         if (tmprc)
>                                 cifs_dbg(FYI, "CIFSCheckMFSymlink: %d
>         \n", tmprc);
>                 }
>         @@ -799,7 +800,8 @@ cifs_get_inode_info(struct inode **inode,
>         const char *full_path,
>         
>                 /* check for Minshall+French symlinks */
>                 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
>         {
>         -               tmprc = CIFSCheckMFSymlink(&fattr, full_path,
>         cifs_sb, xid);
>         +               tmprc = CIFSCheckMFSymlink(xid, tcon, cifs_sb,
>         &fattr,
>         +                                          full_path);
>                         if (tmprc)
>                                 cifs_dbg(FYI, "CIFSCheckMFSymlink: %d
>         \n", tmprc);
>                 }
>         diff --git a/fs/cifs/link.c b/fs/cifs/link.c
>         index cc02347..92aee08 100644
>         --- a/fs/cifs/link.c
>         +++ b/fs/cifs/link.c
>         @@ -354,34 +354,30 @@ open_query_close_cifs_symlink(const
>         unsigned char *path, char *pbuf,
>         
>         
>          int
>         -CIFSCheckMFSymlink(struct cifs_fattr *fattr,
>         -                  const unsigned char *path,
>         -                  struct cifs_sb_info *cifs_sb, unsigned int
>         xid)
>         +CIFSCheckMFSymlink(unsigned int xid, struct cifs_tcon *tcon,
>         +                  struct cifs_sb_info *cifs_sb, struct
>         cifs_fattr *fattr,
>         +                  const unsigned char *path)
>          {
>         -       int rc = 0;
>         +       int rc;
>                 u8 *buf = NULL;
>                 unsigned int link_len = 0;
>                 unsigned int bytes_read = 0;
>         -       struct cifs_tcon *ptcon;
>         
>                 if (!CIFSCouldBeMFSymlink(fattr))
>                         /* it's not a symlink */
>                         return 0;
>         
>                 buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
>         -       if (!buf) {
>         -               rc = -ENOMEM;
>         -               goto out;
>         -       }
>         +       if (!buf)
>         +               return -ENOMEM;
>         
>         -       ptcon = tlink_tcon(cifs_sb_tlink(cifs_sb));
>         -       if ((ptcon->ses) &&
>         (ptcon->ses->server->ops->query_mf_symlink))
>         -               rc =
>         ptcon->ses->server->ops->query_mf_symlink(path, buf,
>         -                                                &bytes_read,
>         cifs_sb, xid);
>         +       if (tcon->ses->server->ops->query_mf_symlink)
>         +               rc =
>         tcon->ses->server->ops->query_mf_symlink(path, buf,
>         +                                               &bytes_read,
>         cifs_sb, xid);
>                 else
>         -               goto out;
>         +               rc = -ENOSYS;
>         
>         -       if (rc != 0)
>         +       if (rc)
>                         goto out;
>         
>                 if (bytes_read == 0) /* not a symlink */
>         --
>         1.8.3.1
>         
>         --
>         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
> 
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH 0/8] Clean up MF-Symlink code and add readlink support for DFS shares

[Steve French]

remaining 7 merged into cifs-2.6.git for-next

On Mon, Nov 25, 2013 at 11:09 AM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> The following set of patches was created as a result of an error
> reported by Red Hat QA which requires code to allow us to call
> readlink on dfs shares.
>
> Along with a fix for this issue, it appeared to be a good time to
> clean up the M-F symlink code to make it protocol agnostic as well
> as fix a couple of bugs in it.
>
> Sachin Prabhu (8):
>   cifs: We do not drop reference to tlink in  CIFSCheckMFSymlink()
>   cifs: Rename and cleanup open_query_close_cifs_symlink()
>   cifs: Rename MF symlink function names
>   cifs: use protocol specific call for query_mf_symlink()
>   cifs: Add create MFSymlinks to protocol ops struct
>   cifs: Re-order M-F Symlink code
>   cifs: move unix extension call to cifs_query_symlink()
>   cifs: Add support for readlink on dfs shares under posix extensions
>
>  fs/cifs/cifsglob.h  |   8 +-
>  fs/cifs/cifsproto.h |  20 ++--
>  fs/cifs/inode.c     |  10 +-
>  fs/cifs/link.c      | 315 ++++++++++++++++++++++++----------------------------
>  fs/cifs/readdir.c   |   2 +-
>  fs/cifs/smb1ops.c   |  55 +++++++--
>  6 files changed, 218 insertions(+), 192 deletions(-)
>
> --
> 1.8.3.1
>
> --
> 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



-- 
Thanks,

Steve