Patchwork cifs: allow calling cifs_build_path_to_root on incomplete cifs_sb

login
register
mail settings
Submitter Jeff Layton
Date Dec. 6, 2010, 12:13 p.m.
Message ID <1291637583-10934-1-git-send-email-jlayton@redhat.com>
Download mbox | patch
Permalink /patch/74339/
State New
Headers show

Comments

Jeff Layton - Dec. 6, 2010, 12:13 p.m.
It's possible that cifs_mount will call cifs_build_path_to_root on a
newly instantiated cifs_sb. In that case, it's likely that the
master_tlink pointer has not yet been instantiated.

Fix this by having cifs_build_path_to_root take a cifsTconInfo pointer
as well, and have the caller pass that in.

Reported-by: Robbert Kouprie <robbert@exx.nl>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/cifsproto.h |    3 ++-
 fs/cifs/connect.c   |    2 +-
 fs/cifs/inode.c     |    6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)
Darren Williams - Feb. 16, 2011, 8:22 a.m.
Hi All

Robert and Jeff thanks for working on and fixing this, I can confirm 
that we now have;
- Remote DFS mounts working, using
- both sec=krb5 and sec=ntlm.

I'm just looking for clarification on the following feature or bug:

we have a SAMBA server, samba1, serving a DFS root as follows

[dfs]
    path = /export/samba/dfs
    msdfs root = yes

configured as:

ls -l /export/samba/dfs
total 8
lrwxrwxrwx 1 root root 79 2010-03-10 07:53 home -> 
msdfs:samba1\dfshome,samba2\dfshome

and SAMBA servers, samba1 and samba2 with shares:

[dfshome]
    comment = DFS Linux Home Directories
    path = /export/home/1/%u
    create mask = 0700
    force create mode = 0700
    directory mask = 0700
    force directory mode = 0700
    read only = No
    browseable = No
    valid users = %U
    map acl inherit = yes



If I cifs mount the DFS root using
mount.cifs //samba1/dfs /tmp/dfs -o <options as required>
then list the directory I get the following errors:


root@atp-h5gcn1s:~# mount.cifs //samba1/dfs /tmp/dfs/ -o <options>
Password:
root@atp-h5gcn1s:~# ls -l /tmp/dfs/
ls: cannot read symbolic link /tmp/dfs/home: Object is remote
ls: cannot read symbolic link /tmp/dfs/test-home: Object is remote
ls: cannot read symbolic link /tmp/dfs/projects: Object is remote
total 8
lrwxrwxrwx 1 root root 79 Mar 10  2010 home
lrwxrwxrwx 1 root root 46 Mar 10  2010 projects
lrwxrwxrwx 1 root root 79 Mar 25  2010 test-home
root@atp-h5gcn1s:~# ls -l /tmp/dfs/home/
ls: cannot access /tmp/dfs/home/: Not a directory
root@atp-h5gcn1s:~# umount /tmp/dfs

I then remount and access a file in 'home' directly, which then allows 
the directory listing to succeed.

root@atp-h5gcn1s:~# mount.cifs //samba1/dfs /tmp/dfs/ -o 
user=dwilliams,dom=NICTA
Password:
root@atp-h5gcn1s:~# ls -l /tmp/dfs/home/.bashrc
-rw-r--r-- 1 <user> <group> 2450 Jun  8  2008 /tmp/dfs/home/.bashrc
root@atp-h5gcn1s:~# ls  /tmp/dfs/
home  projects    test-home
root@atp-h5gcn1s:~# ls -l /tmp/dfs/home/
-rw-r--r--  1 <user> <group>     3078 Aug 17  2007 2007-08-17-P.....
....

also access is OK if you mount 'home' directly via dfs

root@atp-h5gcn1s:~# mount.cifs //samba1/dfs/home /tmp/dfs/ -o <options>

I'm looking for clarification on what the correct operation should be 
since our Windows clients can access the root the traverse the directory 
tree without error, not that this is evidence for correct operation.

Regards
Darren


On 06/12/10 23:13, Jeff Layton wrote:
> It's possible that cifs_mount will call cifs_build_path_to_root on a
> newly instantiated cifs_sb. In that case, it's likely that the
> master_tlink pointer has not yet been instantiated.
>
> Fix this by having cifs_build_path_to_root take a cifsTconInfo pointer
> as well, and have the caller pass that in.
>
> Reported-by: Robbert Kouprie<robbert@exx.nl>
> Signed-off-by: Jeff Layton<jlayton@redhat.com>
> ---
>   fs/cifs/cifsproto.h |    3 ++-
>   fs/cifs/connect.c   |    2 +-
>   fs/cifs/inode.c     |    6 +++---
>   3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index f4cc34f..4e65b67 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -54,7 +54,8 @@ do {								\
>   	     __func__, curr_xid, (int)rc);			\
>   } while (0)
>   extern char *build_path_from_dentry(struct dentry *);
> -extern char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb);
> +extern char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb,
> +					struct cifsTconInfo *tcon);
>   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 af7fa78..f2f58e3 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2913,7 +2913,7 @@ remote_path_check:
>   	/* check if a whole path (including prepath) is not remote */
>   	if (!rc&&  cifs_sb->prepathlen&&  tcon) {
>   		/* build_path_to_root works only when we have a valid tcon */
> -		full_path = cifs_build_path_to_root(cifs_sb);
> +		full_path = cifs_build_path_to_root(cifs_sb, tcon);
>   		if (full_path == NULL) {
>   			rc = -ENOMEM;
>   			goto mount_fail_check;
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 0c1efbb..0023146 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -729,12 +729,12 @@ static const struct inode_operations cifs_ipc_inode_ops = {
>   	.lookup = cifs_lookup,
>   };
>
> -char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb)
> +char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb,
> +				struct cifsTconInfo *tcon)
>   {
>   	int pplen = cifs_sb->prepathlen;
>   	int dfsplen;
>   	char *full_path = NULL;
> -	struct cifsTconInfo *tcon = cifs_sb_master_tcon(cifs_sb);
>
>   	/* if no prefix path, simply set path to the root of share to "" */
>   	if (pplen == 0) {
> @@ -881,7 +881,7 @@ struct inode *cifs_root_iget(struct super_block *sb, unsigned long ino)
>   	char *full_path;
>   	struct cifsTconInfo *tcon = cifs_sb_master_tcon(cifs_sb);
>
> -	full_path = cifs_build_path_to_root(cifs_sb);
> +	full_path = cifs_build_path_to_root(cifs_sb, tcon);
>   	if (full_path == NULL)
>   		return ERR_PTR(-ENOMEM);
>
>

Patch

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index f4cc34f..4e65b67 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -54,7 +54,8 @@  do {								\
 	     __func__, curr_xid, (int)rc);			\
 } while (0)
 extern char *build_path_from_dentry(struct dentry *);
-extern char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb);
+extern char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb,
+					struct cifsTconInfo *tcon);
 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 af7fa78..f2f58e3 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2913,7 +2913,7 @@  remote_path_check:
 	/* check if a whole path (including prepath) is not remote */
 	if (!rc && cifs_sb->prepathlen && tcon) {
 		/* build_path_to_root works only when we have a valid tcon */
-		full_path = cifs_build_path_to_root(cifs_sb);
+		full_path = cifs_build_path_to_root(cifs_sb, tcon);
 		if (full_path == NULL) {
 			rc = -ENOMEM;
 			goto mount_fail_check;
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 0c1efbb..0023146 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -729,12 +729,12 @@  static const struct inode_operations cifs_ipc_inode_ops = {
 	.lookup = cifs_lookup,
 };
 
-char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb)
+char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb,
+				struct cifsTconInfo *tcon)
 {
 	int pplen = cifs_sb->prepathlen;
 	int dfsplen;
 	char *full_path = NULL;
-	struct cifsTconInfo *tcon = cifs_sb_master_tcon(cifs_sb);
 
 	/* if no prefix path, simply set path to the root of share to "" */
 	if (pplen == 0) {
@@ -881,7 +881,7 @@  struct inode *cifs_root_iget(struct super_block *sb, unsigned long ino)
 	char *full_path;
 	struct cifsTconInfo *tcon = cifs_sb_master_tcon(cifs_sb);
 
-	full_path = cifs_build_path_to_root(cifs_sb);
+	full_path = cifs_build_path_to_root(cifs_sb, tcon);
 	if (full_path == NULL)
 		return ERR_PTR(-ENOMEM);