All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aurélien Aptel" <aaptel-IBi9RG/b67k@public.gmane.org>
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH] Add full_path_type arg to cifs_build_path_to_root()
Date: Wed, 20 Apr 2016 18:16:34 +0200	[thread overview]
Message-ID: <20160420181634.1a9ed866@aaptelpc> (raw)


[-- Attachment #1.1: Type: text/plain, Size: 1653 bytes --]

Hi,

When cifs_get_root() calls cifs_build_path_to_root(), it expects a full
path from the root, even in the presence of a DFS link.

e.g. in the case of a DFS link like

    //A/shareA/link -> //B/shareB/sub/dir/

When mounting shareA and doing a "cd link", cifs_get_root() was getting

    "//B/shareB//sub/dir"

Instead of

    "/sub/dir"

Resulting in

    sh: cd: link: No such file or directory

When the DFS link points to a share without a sub-dir suffix
(e.g. //A/shareA/link -> //B/shareB/) cifs_build_path_to_root() returns
an empty string which seems inconsistent. I would expect it have the
same behaviour as when there is a sub-dir and prefix the whole DFS UNC
path (it returns "" where I would expect "//B/shareB/"). This might be
a bug? I will try to return the expected path to see what happens.

I would like to know what you think.

There are 2 ways to fix this:
- never prefix an UNC path, even when using a DFS link i.e. remove all
  code dealing with DFS from cifs_build_path_to_root)
- add a new arg to decide the behaviour of cifs_build_path_to_root().

The attached patch implements the second solution and fixes the problem.
- lets the caller of cifs_build_path_to_root() decide the behaviour via
  a new argument.
- make cifs_get_root() always use a simple path.
- keep old behaviour on the other code paths (only one, in
  cifs_do_mount)

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-fs-cifs-Add-full_path_type-enum.patch --]
[-- Type: text/x-patch, Size: 855 bytes --]

From 3f877b42d8d2850b4c384e0c394dc7b71fca491b Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
Date: Wed, 20 Apr 2016 16:32:46 +0200
Subject: [PATCH 1/2] fs/cifs: Add full_path_type enum

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

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index f2cc0b3..2b5b796 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1644,4 +1644,10 @@ extern struct smb_version_values smb302_values;
 #define ALT_SMB311_VERSION_STRING "3.11"
 extern struct smb_version_operations smb311_operations;
 extern struct smb_version_values smb311_values;
+
+enum full_path_type {
+	CIFS_NO_PREFIX,
+	CIFS_WITH_DFS_UNC_PREFIX,
+};
+
 #endif	/* _CIFS_GLOB_H */
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-fs-cifs-Add-full_path_type-arg-to-cifs_build_path_to.patch --]
[-- Type: text/x-patch, Size: 3991 bytes --]

From 1caea1e53763df316b6c7b4cc9dc7b54c3f3300e Mon Sep 17 00:00:00 2001
From: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
Date: Wed, 20 Apr 2016 16:34:01 +0200
Subject: [PATCH 2/2] fs/cifs: Add full_path_type arg to
 cifs_build_path_to_root()

Only add the server prefix in the case of a DFS link when explicitely
asked.

This function is called from:
- cifs_mount()
- cifs_get_root()

The later expects a full path from the root, even in the presence of a
DFS link.

e.g. in the case of a DFS link like

    //A/shareA/link -> //B/shareB/sub/dir/

When doing a "cd link", cifs_get_root() was getting

    "//B/shareB//sub/dir"

Instead of

    "/sub/dir"

Resulting in

    sh: cd: link: No such file or directory

This commit
- lets the caller of cifs_build_path_to_root() decide the behaviour via
  a new argument.
- make cifs_get_root() always use a simple path.

Thanks to Josef Cejka <jcejka-IBi9RG/b67k@public.gmane.org> for finding the bug&fix.

Reported-by: Fons Jongh <fons.dejongh-/y5eVf3Am6FByuSxxbvQtw@public.gmane.org>
Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
---
 fs/cifs/cifsfs.c    | 3 ++-
 fs/cifs/cifsproto.h | 3 ++-
 fs/cifs/connect.c   | 2 +-
 fs/cifs/dir.c       | 6 ++++--
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 8920156..d56a5ed 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -607,7 +607,8 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
 	char sep;
 
 	full_path = cifs_build_path_to_root(vol, cifs_sb,
-					    cifs_sb_master_tcon(cifs_sb));
+					    cifs_sb_master_tcon(cifs_sb), CIFS_NO_PREFIX);
+
 	if (full_path == NULL)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index eed7ff50..f6a0902 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -63,7 +63,8 @@ extern void exit_cifs_idmap(void);
 extern char *build_path_from_dentry(struct dentry *);
 extern char *cifs_build_path_to_root(struct smb_vol *vol,
 				     struct cifs_sb_info *cifs_sb,
-				     struct cifs_tcon *tcon);
+				     struct cifs_tcon *tcon,
+	                             enum full_path_type);
 extern char *build_wildcard_path_from_dentry(struct dentry *direntry);
 extern char *cifs_compose_mount_options(const char *sb_mountdata,
 		const char *fullpath, const struct dfs_info3_param *ref,
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 6f62ac8..b054218 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3660,7 +3660,7 @@ remote_path_check:
 		/*
 		 * cifs_build_path_to_root works only when we have a valid tcon
 		 */
-		full_path = cifs_build_path_to_root(volume_info, cifs_sb, tcon);
+		full_path = cifs_build_path_to_root(volume_info, cifs_sb, tcon, CIFS_SERVER_PREFIX);
 		if (full_path == NULL) {
 			rc = -ENOMEM;
 			goto mount_fail_check;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index c3eb998..b5fe596 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -47,7 +47,7 @@ renew_parental_timestamps(struct dentry *direntry)
 
 char *
 cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
-			struct cifs_tcon *tcon)
+			struct cifs_tcon *tcon, enum full_path_type prefix)
 {
 	int pplen = vol->prepath ? strlen(vol->prepath) + 1 : 0;
 	int dfsplen;
@@ -59,7 +59,8 @@ cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
 		return full_path;
 	}
 
-	if (tcon->Flags & SMB_SHARE_IS_IN_DFS)
+
+	if (prefix == CIFS_WITH_DFS_UNC_PREFIX && (tcon->Flags & SMB_SHARE_IS_IN_DFS))
 		dfsplen = strnlen(tcon->treeName, MAX_TREE_SIZE + 1);
 	else
 		dfsplen = 0;
@@ -68,6 +69,7 @@ cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb,
 	if (full_path == NULL)
 		return full_path;
 
+
 	if (dfsplen)
 		strncpy(full_path, tcon->treeName, dfsplen);
 	full_path[dfsplen] = CIFS_DIR_SEP(cifs_sb);
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

             reply	other threads:[~2016-04-20 16:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-20 16:16 Aurélien Aptel [this message]
2016-08-01 12:32 ` [PATCH] Add full_path_type arg to cifs_build_path_to_root() Aurélien Aptel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160420181634.1a9ed866@aaptelpc \
    --to=aaptel-ibi9rg/b67k@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.