dfs path construction fixup for / character in \\server\share component of dfs path

dfs path construction fixup for / character in \\server\share component of dfs path

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

Attached is dfs path construction fixup for / character in
\\server\share component of dfs path.  Let me know if you see any
problem - but it gets Samba past the problem with not returning
STATUS_PATH_NOT_COVERED

similar change will have to be made to build_full_dfs_path_from_dentry
in cifs_dfs_ref.c

This makes it easier for me to test the remainder of the changes
needed to the SMB GetDFSReferral function(s).  Has anyone checked on
the answer to Al's question on submount expiry from a few days ago?

-- 
Thanks,

Steve

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dfs-slash-in-unc.patch --]
[-- Type: text/x-diff; name=dfs-slash-in-unc.patch, Size: 1768 bytes --]

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index bc673c8..e1031b9 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -161,12 +161,14 @@ static void cifs_unix_info_to_inode(struct inode *inode,
 	spin_unlock(&inode->i_lock);
 }
 
-static const unsigned char *cifs_get_search_path(struct cifsTconInfo *pTcon,
-					const char *search_path)
+static const unsigned char *cifs_get_search_path(struct cifs_sb_info *cifs_sb,
+						const char *search_path)
 {
 	int tree_len;
 	int path_len;
+	int i;
 	char *tmp_path;
+	struct cifsTconInfo *pTcon = cifs_sb->tcon;
 
 	if (!(pTcon->Flags & SMB_SHARE_IS_IN_DFS))
 		return search_path;
@@ -180,6 +182,11 @@ static const unsigned char *cifs_get_search_path(struct cifsTconInfo *pTcon,
 		return search_path;
 
 	strncpy(tmp_path, pTcon->treeName, tree_len);
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)
+		for (i = 0; i < tree_len; i++) {
+			if (tmp_path[i] == '\\')
+				tmp_path[i] = '/';
+		}
 	strncpy(tmp_path+tree_len, search_path, path_len);
 	tmp_path[tree_len+path_len] = 0;
 	return tmp_path;
@@ -199,7 +206,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 	pTcon = cifs_sb->tcon;
 	cFYI(1, ("Getting info on %s", search_path));
 
-	full_path = cifs_get_search_path(pTcon, search_path);
+	full_path = cifs_get_search_path(cifs_sb, search_path);
 
 try_again_CIFSSMBUnixQPathInfo:
 	/* could have done a find first instead but this returns more info */
@@ -402,7 +409,7 @@ int cifs_get_inode_info(struct inode **pinode,
 			return -ENOMEM;
 		pfindData = (FILE_ALL_INFO *)buf;
 
-		full_path = cifs_get_search_path(pTcon, search_path);
+		full_path = cifs_get_search_path(cifs_sb, search_path);
 
 try_again_CIFSSMBQPathInfo:
 		/* could do find first instead but this returns more info */

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Igor Mammedov]

[-- Attachment #1: Type: text/plain, Size: 3806 bytes --]

Steve French wrote:
> Attached is dfs path construction fixup for / character in
> \\server\share component of dfs path.  Let me know if you see any
> problem - but it gets Samba past the problem with not returning
> STATUS_PATH_NOT_COVERED
> 
> similar change will have to be made to build_full_dfs_path_from_dentry
> in cifs_dfs_ref.c
> 
> This makes it easier for me to test the remainder of the changes
> needed to the SMB GetDFSReferral function(s).  Has anyone checked on
> the answer to Al's question on submount expiry from a few days ago?


I've just tested it and the patch fixed a problem with the lack of 
STATUS_PATH_NOT_COVERED with unix-ext enabled on samba server.
1. samba with unix-ext enabled returns (5th packet) dfs link as a symbolic link,
but for dfs code it should be the directory type so we could fix it in the time 
of creating inode and get the server generated inode number.

No.     Time        Source                Destination           Protocol Info
      1 0.000000    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/dfs2
      2 0.000385    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_PATH_NOT_COVERED
      3 0.000670    192.168.133.129       192.168.133.1         TCP      46662 > microsoft-ds [ACK] Seq=127 Ack=40 Win=1728 Len=0 TSV=509648098 TSER=3625842022

      4 0.006911    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: /dfs2
      5 0.007110    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO

                                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - here our chance to get it working

      6 0.016002    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Link, Path: /dfs2
      7 0.016219    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
      8 0.049621    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/msdfs:\172.16.61.1\dfs2
      9 0.050706    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND

Patches that make dfs working in this case are attached.



2. samba with unix-ext disabled returns STATUS_OBJECT_NAME_NOT_FOUND on the second
query (5th packet). Replacing '\' with '/' doesn't help. It looks we will not be 
able to make this case work unless we disable server generated inode numbers and fill
up junction point inode with fake values. Another way to make it working is to fix
samba code so that it would return inode info (with directory type) on the second call
as win2k does.

No.     Time        Source                Destination           Protocol Info
      1 0.000000    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File All Info, Path: \\192.168.133.1\dfs\dfs2
      2 0.000576    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_PATH_NOT_COVERED
      3 0.000746    192.168.133.129       192.168.133.1         TCP      37925 > microsoft-ds [ACK] Seq=127 Ack=40 Win=1728 Len=0 TSV=510792731 TSER=3626976733

      4 0.004212    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File All Info, Path: \dfs2
      5 0.004606    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND

-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com





[-- Attachment #2: 0001-Adds-to-dns_resolver-checking-if-the-server-name-is.patch --]
[-- Type: text/x-patch, Size: 2721 bytes --]

>From deef01fdc817a0e0d167e28eabe30890fc789608 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <niallain@gmail.com>
Date: Wed, 23 Apr 2008 17:34:28 +0400
Subject: [PATCH] Adds to dns_resolver checking if the server name is an IP addr and skipping upcall in this case.

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 fs/cifs/dns_resolve.c |   62 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/dns_resolve.c b/fs/cifs/dns_resolve.c
index 7cc86c4..939e256 100644
--- a/fs/cifs/dns_resolve.c
+++ b/fs/cifs/dns_resolve.c
@@ -55,6 +55,32 @@ struct key_type key_type_dns_resolver = {
 	.match       = user_match,
 };
 
+/* Checks if supplied name is IP address
+ * returns:
+ * 		1 - name is IP
+ * 		0 - name is not IP
+ */
+static int is_ip(const char *name)
+{
+	int rc;
+	struct sockaddr_in sin_server;
+	struct sockaddr_in6 sin_server6;
+
+	rc = cifs_inet_pton(AF_INET, name,
+			&sin_server.sin_addr.s_addr);
+
+	if (rc <= 0) {
+		/* not ipv4 address, try ipv6 */
+		rc = cifs_inet_pton(AF_INET6, name,
+				&sin_server6.sin6_addr.in6_u);
+		if (rc > 0)
+			return 1;
+	} else {
+		return 1;
+	}
+	/* we failed translating address */
+	return 0;
+}
 
 /* Resolves server name to ip address.
  * input:
@@ -67,8 +93,9 @@ int
 dns_resolve_server_name_to_ip(const char *unc, char **ip_addr)
 {
 	int rc = -EAGAIN;
-	struct key *rkey;
+	struct key *rkey = ERR_PTR(-EAGAIN);
 	char *name;
+	char *data = NULL;
 	int len;
 
 	if (!ip_addr || !unc)
@@ -97,26 +124,41 @@ dns_resolve_server_name_to_ip(const char *unc, char **ip_addr)
 	memcpy(name, unc+2, len);
 	name[len] = 0;
 
+	if (is_ip(name)) {
+		cFYI(1, ("%s: it is IP, skipping dns upcall: %s",
+					__func__, name));
+		data = name;
+		goto skip_upcall;
+	}
+
 	rkey = request_key(&key_type_dns_resolver, name, "");
 	if (!IS_ERR(rkey)) {
-		len = strlen(rkey->payload.data);
-		*ip_addr = kmalloc(len+1, GFP_KERNEL);
-		if (*ip_addr) {
-			memcpy(*ip_addr, rkey->payload.data, len);
-			(*ip_addr)[len] = '\0';
-			cFYI(1, ("%s: resolved: %s to %s", __func__,
+		data = rkey->payload.data;
+		cFYI(1, ("%s: resolved: %s to %s", __func__,
 					rkey->description,
 					*ip_addr
 				));
+	} else {
+		cERROR(1, ("%s: unable to resolve: %s", __func__, name));
+		goto out;
+	}
+
+skip_upcall:
+	if (data) {
+		len = strlen(data);
+		*ip_addr = kmalloc(len+1, GFP_KERNEL);
+		if (*ip_addr) {
+			memcpy(*ip_addr, data, len);
+			(*ip_addr)[len] = '\0';
 			rc = 0;
 		} else {
 			rc = -ENOMEM;
 		}
-		key_put(rkey);
-	} else {
-		cERROR(1, ("%s: unable to resolve: %s", __func__, name));
+		if (!IS_ERR(rkey))
+			key_put(rkey);
 	}
 
+out:
 	kfree(name);
 	return rc;
 }
-- 
1.5.3.7


[-- Attachment #3: 0002-fixed-compatibility-issue-with-samba-a-refferal-req.patch --]
[-- Type: text/x-patch, Size: 2244 bytes --]

>From d69f4d0a59086658491eec4f2e43a890c4357a29 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <niallain@gmail.com>
Date: Wed, 23 Apr 2008 17:40:34 +0400
Subject: [PATCH] fixed compatibility issue with samba: a refferal request UNC is canonized to '/' path separator

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 fs/cifs/cifs_dfs_ref.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index f53f41f..bc110f7 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -81,15 +81,11 @@ static char *cifs_get_share_name(const char *node_name)
 	/* find sharename end */
 	pSep++;
 	pSep = memchr(UNC+(pSep-UNC), '\\', len-(pSep-UNC));
-	if (!pSep) {
-		cERROR(1, ("%s:2 cant find share name in node name: %s",
-			__func__, node_name));
-		kfree(UNC);
-		return NULL;
+	if (pSep) {
+		/* trim path up to sharename end
+		 * now we have share name in UNC */
+		*pSep = 0;
 	}
-	/* trim path up to sharename end
-	 *          * now we have share name in UNC */
-	*pSep = 0;
 
 	return UNC;
 }
@@ -176,7 +172,7 @@ static char *compose_mount_options(const char *sb_mountdata,
 		tkn_e = strchr(tkn_e+1, '\\');
 		if (tkn_e) {
 			strcat(mountdata, ",prefixpath=");
-			strcat(mountdata, tkn_e);
+			strcat(mountdata, tkn_e+1);
 		}
 	}
 
@@ -232,7 +228,8 @@ static char *build_full_dfs_path_from_dentry(struct dentry *dentry)
 		return NULL;
 
 	if (cifs_sb->tcon->Flags & SMB_SHARE_IS_IN_DFS) {
-		/* we should use full path name to correct working with DFS */
+		int i;
+		/* we should use full path name for correct working with DFS */
 		l_max_len = strnlen(cifs_sb->tcon->treeName, MAX_TREE_SIZE+1) +
 					strnlen(search_path, MAX_PATHCONF) + 1;
 		tmp_path = kmalloc(l_max_len, GFP_KERNEL);
@@ -243,6 +240,12 @@ static char *build_full_dfs_path_from_dentry(struct dentry *dentry)
 		strncpy(tmp_path, cifs_sb->tcon->treeName, l_max_len);
 		strcat(tmp_path, search_path);
 		tmp_path[l_max_len-1] = 0;
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS)
+			for (i = 0; i < l_max_len; i++) {
+				if (tmp_path[i] == '\\')
+					tmp_path[i] = '/';
+			}
+
 		full_path = tmp_path;
 		kfree(search_path);
 	} else {
-- 
1.5.3.7


[-- Attachment #4: 0003-Workaround-for-working-with-samba-unix-extentions-m.patch --]
[-- Type: text/x-patch, Size: 1406 bytes --]

>From e5914b5297d953ba3d08fb79bb82e0d890130218 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <niallain@gmail.com>
Date: Wed, 23 Apr 2008 17:57:34 +0400
Subject: [PATCH] Workaround for working with samba (unix extentions mode enabled only).

Fixup inode from symlink to directory mode, so we could mount on it
a dfs submount.

Since samba in unix-ext mode returns a dfs junction point as a link,
we have a chance to get symlink info (including inode number) and
right after that fix inode mode from symlink to directory.
That looks wierd but DFS works (at least in the case when samba is
in unix-ext mode).`

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 fs/cifs/inode.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index e1031b9..144879b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -261,6 +261,14 @@ try_again_CIFSSMBUnixQPathInfo:
 
 		cifs_unix_info_to_inode(inode, &findData, 0);
 
+		/* if it dfs link (i.e. we've got PATH_UNCOVERED error)
+		 * we need to fix up its type to the directory before
+		 * making it mount point */
+		if (is_dfs_referral && (inode->i_mode & S_IFMT & S_IFLNK)) {
+			inode->i_mode &= ~S_IFLNK;
+			inode->i_mode |= S_IFDIR;
+			cFYI(1, ("faking dir type for the dfs link"));
+		}
 
 		if (num_of_bytes < end_of_file)
 			cFYI(1, ("allocation size less than end of file"));
-- 
1.5.3.7

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Wed, Apr 23, 2008 at 06:28:39PM +0400, Igor Mammedov wrote:
> Steve French wrote:
> > Attached is dfs path construction fixup for / character in
> > \\server\share component of dfs path.  Let me know if you see any
> > problem - but it gets Samba past the problem with not returning
> > STATUS_PATH_NOT_COVERED
> > 
> > similar change will have to be made to build_full_dfs_path_from_dentry
> > in cifs_dfs_ref.c
> > 
> > This makes it easier for me to test the remainder of the changes
> > needed to the SMB GetDFSReferral function(s).  Has anyone checked on
> > the answer to Al's question on submount expiry from a few days ago?
> 
> 
> I've just tested it and the patch fixed a problem with the lack of 
> STATUS_PATH_NOT_COVERED with unix-ext enabled on samba server.
> 1. samba with unix-ext enabled returns (5th packet) dfs link as a symbolic link,
> but for dfs code it should be the directory type so we could fix it in the time 
> of creating inode and get the server generated inode number.
> 
> No.     Time        Source                Destination           Protocol Info
>       1 0.000000    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/dfs2
>       2 0.000385    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_PATH_NOT_COVERED
>       3 0.000670    192.168.133.129       192.168.133.1         TCP      46662 > microsoft-ds [ACK] Seq=127 Ack=40 Win=1728 Len=0 TSV=509648098 TSER=3625842022
> 
>       4 0.006911    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: /dfs2
>       5 0.007110    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
> 
>                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - here our chance to get it working
> 
>       6 0.016002    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Link, Path: /dfs2
>       7 0.016219    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
>       8 0.049621    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/msdfs:\172.16.61.1\dfs2
>       9 0.050706    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND
> 
> Patches that make dfs working in this case are attached.

Thanks. I'm going to fix Samba 3.2 (not sure if this will make
final release) to return directory not symlink on QPATHINFO
if the requested path is a DFS path and it's a DFS link.

Can you test the patch once it's complete please ?

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Wed, Apr 23, 2008 at 12:11:50PM -0700, Jeremy Allison wrote:
> On Wed, Apr 23, 2008 at 06:28:39PM +0400, Igor Mammedov wrote:
> > Steve French wrote:
> > > Attached is dfs path construction fixup for / character in
> > > \\server\share component of dfs path.  Let me know if you see any
> > > problem - but it gets Samba past the problem with not returning
> > > STATUS_PATH_NOT_COVERED
> > > 
> > > similar change will have to be made to build_full_dfs_path_from_dentry
> > > in cifs_dfs_ref.c
> > > 
> > > This makes it easier for me to test the remainder of the changes
> > > needed to the SMB GetDFSReferral function(s).  Has anyone checked on
> > > the answer to Al's question on submount expiry from a few days ago?
> > 
> > 
> > I've just tested it and the patch fixed a problem with the lack of 
> > STATUS_PATH_NOT_COVERED with unix-ext enabled on samba server.
> > 1. samba with unix-ext enabled returns (5th packet) dfs link as a symbolic link,
> > but for dfs code it should be the directory type so we could fix it in the time 
> > of creating inode and get the server generated inode number.
> > 
> > No.     Time        Source                Destination           Protocol Info
> >       1 0.000000    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/dfs2
> >       2 0.000385    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_PATH_NOT_COVERED
> >       3 0.000670    192.168.133.129       192.168.133.1         TCP      46662 > microsoft-ds [ACK] Seq=127 Ack=40 Win=1728 Len=0 TSV=509648098 TSER=3625842022
> > 
> >       4 0.006911    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: /dfs2
> >       5 0.007110    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
> > 
> >                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - here our chance to get it working
> > 
> >       6 0.016002    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Link, Path: /dfs2
> >       7 0.016219    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
> >       8 0.049621    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/msdfs:\172.16.61.1\dfs2
> >       9 0.050706    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND
> > 
> > Patches that make dfs working in this case are attached.
> 
> Thanks. I'm going to fix Samba 3.2 (not sure if this will make
> final release) to return directory not symlink on QPATHINFO

Hmmmmm. Looking carefully that's the wrong thing to do.

I think the client is doing the wrong thing here when it
gets the STATUS_PATH_NOT_COVERED error. Shouldn't it then
call TRANS2_GET_DFS_REFERRAL instead of doing a QPATHINFO ?

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Igor Mammedov]

Jeremy Allison wrote:
> On Wed, Apr 23, 2008 at 12:11:50PM -0700, Jeremy Allison wrote:
>> On Wed, Apr 23, 2008 at 06:28:39PM +0400, Igor Mammedov wrote:
>>> Steve French wrote:
>>>> Attached is dfs path construction fixup for / character in
>>>> \\server\share component of dfs path.  Let me know if you see any
>>>> problem - but it gets Samba past the problem with not returning
>>>> STATUS_PATH_NOT_COVERED
>>>>
>>>> similar change will have to be made to build_full_dfs_path_from_dentry
>>>> in cifs_dfs_ref.c
>>>>
>>>> This makes it easier for me to test the remainder of the changes
>>>> needed to the SMB GetDFSReferral function(s).  Has anyone checked on
>>>> the answer to Al's question on submount expiry from a few days ago?
>>>
>>> I've just tested it and the patch fixed a problem with the lack of 
>>> STATUS_PATH_NOT_COVERED with unix-ext enabled on samba server.
>>> 1. samba with unix-ext enabled returns (5th packet) dfs link as a symbolic link,
>>> but for dfs code it should be the directory type so we could fix it in the time 
>>> of creating inode and get the server generated inode number.
>>>
>>> No.     Time        Source                Destination           Protocol Info
>>>       1 0.000000    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/dfs2
>>>       2 0.000385    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_PATH_NOT_COVERED
>>>       3 0.000670    192.168.133.129       192.168.133.1         TCP      46662 > microsoft-ds [ACK] Seq=127 Ack=40 Win=1728 Len=0 TSV=509648098 TSER=3625842022
>>>
>>>       4 0.006911    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: /dfs2
>>>       5 0.007110    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
>>>
>>>                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - here our chance to get it working
>>>
>>>       6 0.016002    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Link, Path: /dfs2
>>>       7 0.016219    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO
>>>       8 0.049621    192.168.133.129       192.168.133.1         SMB      Trans2 Request, QUERY_PATH_INFO, Query File Unix Basic, Path: //192.168.133.1/dfs/msdfs:\172.16.61.1\dfs2
>>>       9 0.050706    192.168.133.1         192.168.133.129       SMB      Trans2 Response, QUERY_PATH_INFO, Error: STATUS_OBJECT_NAME_NOT_FOUND
>>>
>>> Patches that make dfs working in this case are attached.
>> Thanks. I'm going to fix Samba 3.2 (not sure if this will make
>> final release) to return directory not symlink on QPATHINFO

Can try to do it, have 'v3-2-test' branch checked out already.

> 
> Hmmmmm. Looking carefully that's the wrong thing to do.
> 
> I think the client is doing the wrong thing here when it
> gets the STATUS_PATH_NOT_COVERED error. Shouldn't it then
> call TRANS2_GET_DFS_REFERRAL instead of doing a QPATHINFO ?

I'm doing the second call with a short path to get inode info
including server generated inode number. If not for the last 
then second call could be omitted and inode be filled with fake
values and locally generated ino.

PS:
Windows server does not object against the second call and returns
info on the dfs junction point (as directory). 
More uniform behavior between different implementations would be
better for all.


-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:

> I'm doing the second call with a short path to get inode info
> including server generated inode number. If not for the last 
> then second call could be omitted and inode be filled with fake
> values and locally generated ino.
> 
> PS:
> Windows server does not object against the second call and returns
> info on the dfs junction point (as directory). 
> More uniform behavior between different implementations would be
> better for all.

Very important question here. Do you set the FLAGS2_DFS_PATHNAMES
bit in the flags2 when you do this QPATHINFO call on the short path ?

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Fri, Apr 25, 2008 at 12:22:14PM -0700, Jeremy Allison wrote:
> On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:
> 
> > I'm doing the second call with a short path to get inode info
> > including server generated inode number. If not for the last 
> > then second call could be omitted and inode be filled with fake
> > values and locally generated ino.
> > 
> > PS:
> > Windows server does not object against the second call and returns
> > info on the dfs junction point (as directory). 
> > More uniform behavior between different implementations would be
> > better for all.
> 
> Very important question here. Do you set the FLAGS2_DFS_PATHNAMES
> bit in the flags2 when you do this QPATHINFO call on the short path ?

Actually, all these questions would be answered if you'd just
forward me the binary wireshark trace you have here.

Thanks !

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:

> I'm doing the second call with a short path to get inode info
> including server generated inode number. If not for the last 
> then second call could be omitted and inode be filled with fake
> values and locally generated ino.
> 
> PS:
> Windows server does not object against the second call and returns
> info on the dfs junction point (as directory). 
> More uniform behavior between different implementations would be
> better for all.

Can you try this patch against the 3.2 code please. It should
cause smbd to return a directory on the short QFILEINFO call.

Jeremy.

[-- Attachment #2: look --]
[-- Type: text/plain, Size: 2611 bytes --]

diff --git a/source/smbd/trans2.c b/source/smbd/trans2.c
index 709eb39..74b167b 100644
--- a/source/smbd/trans2.c
+++ b/source/smbd/trans2.c
@@ -3768,6 +3768,28 @@ static void call_trans2qpipeinfo(connection_struct *conn,
 }
 
 /****************************************************************************
+ Needed to show the msdfs symlinks as directories. Modified psbuf
+ to be a directory.
+****************************************************************************/
+
+static bool check_msdfs_link(connection_struct *conn,
+				const char *pathname,
+				SMB_STRUCT_STAT *psbuf)
+{
+	if(lp_host_msdfs() &&
+		lp_msdfs_root(SNUM(conn)) &&
+		is_msdfs_link(conn, pathname, psbuf)) {
+
+		DEBUG(5,("check_msdfs_link: Masquerading msdfs link %s "
+			"as a directory\n",
+			pathname));
+		psbuf->st_mode = (psbuf->st_mode & 0xFFF) | S_IFDIR;
+		return true;
+	}
+	return false;
+}
+
+/****************************************************************************
  Reply to a TRANS2_QFILEPATHINFO or TRANSACT2_QFILEINFO (query file info by
  file name or file id).
 ****************************************************************************/
@@ -3806,6 +3828,7 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
 	struct ea_list *ea_list = NULL;
 	uint32 access_mask = 0x12019F; /* Default - GENERIC_EXECUTE mapping from Windows */
 	char *lock_data = NULL;
+	bool ms_dfs_link = false;
 	TALLOC_CTX *ctx = talloc_tos();
 
 	if (!params) {
@@ -3959,10 +3982,20 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
 				reply_unixerror(req, ERRDOS, ERRbadpath);
 				return;
 			}
+
+			ms_dfs_link = check_msdfs_link(conn,fname,&sbuf);
+
 		} else if (!VALID_STAT(sbuf) && SMB_VFS_STAT(conn,fname,&sbuf) && (info_level != SMB_INFO_IS_NAME_VALID)) {
-			DEBUG(3,("call_trans2qfilepathinfo: SMB_VFS_STAT of %s failed (%s)\n",fname,strerror(errno)));
-			reply_unixerror(req, ERRDOS, ERRbadpath);
-			return;
+			int saved_errno = errno;
+
+			ms_dfs_link = check_msdfs_link(conn,fname,&sbuf);
+
+			if (!ms_dfs_link) {
+				errno = saved_errno;
+				DEBUG(3,("call_trans2qfilepathinfo: SMB_VFS_STAT of %s failed (%s)\n",fname,strerror(errno)));
+				reply_unixerror(req, ERRDOS, ERRbadpath);
+				return;
+			}
 		}
 
 		fileid = vfs_file_id_from_sbuf(conn, &sbuf);
@@ -3987,7 +4020,11 @@ static void call_trans2qfilepathinfo(connection_struct *conn,
 	else
 		base_name = p+1;
 
-	mode = dos_mode(conn,fname,&sbuf);
+	if (ms_dfs_link) {
+		mode = dos_mode_msdfs(conn,fname,&sbuf);
+	} else {
+		mode = dos_mode(conn,fname,&sbuf);
+	}
 	if (!mode)
 		mode = FILE_ATTRIBUTE_NORMAL;
 

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Igor Mammedov]

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

Jeremy Allison wrote:
> On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:
> 
>> I'm doing the second call with a short path to get inode info
>> including server generated inode number. If not for the last 
>> then second call could be omitted and inode be filled with fake
>> values and locally generated ino.
>>
>> PS:
>> Windows server does not object against the second call and returns
>> info on the dfs junction point (as directory). 
>> More uniform behavior between different implementations would be
>> better for all.
> 
> Can you try this patch against the 3.2 code please. It should
> cause smbd to return a directory on the short QFILEINFO call.

Thanks for a patch, I've just tested it. Packets dumps are attached. 

Short summary:
1. unix extentions are disabled.  Works.
   * ls on the directory that has a dfs link "dfs2" shows that it is directory
   * second QPATHINFO on "\dfs2" returns that it is directory (pkts 28-29)

2. unix extentions are enabled. Works partially.
   * ls on the directory that has a dfs link "dfs2" shows that it is a link 
     (pkts 26-27). Would be nice if it was listed as directory here. 
   * second QPATHINFO on "\dfs2" returns that it is directory (pkts 36-37)

Traverse over DFS junction point now works in both both cases.


-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com





[-- Attachment #2: smb32.Jeremy_unix_ext_disabled.pcap --]
[-- Type: application/octet-stream, Size: 9561 bytes --]

[-- Attachment #3: smb32.Jeremy_unix_ext_enabled.pcap --]
[-- Type: application/octet-stream, Size: 14286 bytes --]

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Sun, Apr 27, 2008 at 05:00:20PM +0400, Igor Mammedov wrote:
> Jeremy Allison wrote:
> > On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:
> > 
> >> I'm doing the second call with a short path to get inode info
> >> including server generated inode number. If not for the last 
> >> then second call could be omitted and inode be filled with fake
> >> values and locally generated ino.
> >>
> >> PS:
> >> Windows server does not object against the second call and returns
> >> info on the dfs junction point (as directory). 
> >> More uniform behavior between different implementations would be
> >> better for all.
> > 
> > Can you try this patch against the 3.2 code please. It should
> > cause smbd to return a directory on the short QFILEINFO call.
> 
> Thanks for a patch, I've just tested it. Packets dumps are attached. 
> 
> Short summary:
> 1. unix extentions are disabled.  Works.
>    * ls on the directory that has a dfs link "dfs2" shows that it is directory
>    * second QPATHINFO on "\dfs2" returns that it is directory (pkts 28-29)
> 
> 2. unix extentions are enabled. Works partially.
>    * ls on the directory that has a dfs link "dfs2" shows that it is a link 
>      (pkts 26-27). Would be nice if it was listed as directory here. 
>    * second QPATHINFO on "\dfs2" returns that it is directory (pkts 36-37)
> 
> Traverse over DFS junction point now works in both both cases.

Can you send me the same packet trace against Windows please ? I need
to see exactly how Windows responds to the \dfs2 query with flags2
set.

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Sun, Apr 27, 2008 at 05:00:20PM +0400, Igor Mammedov wrote:
> Jeremy Allison wrote:
> > On Thu, Apr 24, 2008 at 12:04:06PM +0400, Igor Mammedov wrote:
> > 
> >> I'm doing the second call with a short path to get inode info
> >> including server generated inode number. If not for the last 
> >> then second call could be omitted and inode be filled with fake
> >> values and locally generated ino.
> >>
> >> PS:
> >> Windows server does not object against the second call and returns
> >> info on the dfs junction point (as directory). 
> >> More uniform behavior between different implementations would be
> >> better for all.
> > 
> > Can you try this patch against the 3.2 code please. It should
> > cause smbd to return a directory on the short QFILEINFO call.
> 
> Thanks for a patch, I've just tested it. Packets dumps are attached. 
> 
> Short summary:
> 1. unix extentions are disabled.  Works.
>    * ls on the directory that has a dfs link "dfs2" shows that it is directory
>    * second QPATHINFO on "\dfs2" returns that it is directory (pkts 28-29)
> 
> 2. unix extentions are enabled. Works partially.
>    * ls on the directory that has a dfs link "dfs2" shows that it is a link 
>      (pkts 26-27). Would be nice if it was listed as directory here. 
>    * second QPATHINFO on "\dfs2" returns that it is directory (pkts 36-37)
> 
> Traverse over DFS junction point now works in both both cases.

Ok, I just had a long chat with Volker about this and he made
some good points.

With unix extensions enabled doing a QPATHINFO or FINDFIRST
with a UNIX info level on /server/share/dfs/path always
returns PATH_NOT_COVERED, as it should.

Doing a QPATHINFO or FINDFIRST on /dfs/path with a UNIX info
level currently returns symlink, which exposes the Samba
implementation of the junction. For a Windows info level
it returns BAD_PATH, which is a bug in the Samba server
and part of my patch trivially fixes this.

The question I'm pondoring is - is it correct to spoof
this to show a directory when you are querying with a
UNIX info level ? On the positive side it hides the
Samba implementations. On the negative side it makes
it impossible to write code that actually allows the
client to see what is really on disk.

This goes against the spirit of the UNIX extensions,
which is to show straight "posix" on the wire. Now
we are in somewhat unknown territory, in that we're
defining the interface between UNIX extensions and
MSDFS, but if I spoof to show a directory when it's
really a symlink I'm lying to clients.

If I do the spoof there's no way for clients that
want to see the symlink (maybe in order to change
it) to do so.

I could add some "magic foo" and show the symlink
if the QPATHINFO request flags2 doesn't contain the
DFS_PATH bit, and show as a directory if it does,
but this sucks (and I hate magic foo such as this).

I'm inclined as Volker suggested to leave the current
behavior with UNIX extensions alone, as the client
knows it's dealing with a DFS path (after all the
first call got PATH_NOT_COVERED) and it allows the
client to code around the discrepancy (as you say
traverse over DFS junctions now works with the client)
but still allows the UNIX extensions to show what
is really on disk in full POSIX glory :-).

Steve, others, please comment !

Cheers,

	Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Igor Mammedov]

Jeremy Allison wrote:
> Ok, I just had a long chat with Volker about this and he made
> some good points.
> 
> With unix extensions enabled doing a QPATHINFO or FINDFIRST
> with a UNIX info level on /server/share/dfs/path always
> returns PATH_NOT_COVERED, as it should.
> 
> Doing a QPATHINFO or FINDFIRST on /dfs/path with a UNIX info
> level currently returns symlink, which exposes the Samba
> implementation of the junction. For a Windows info level
> it returns BAD_PATH, which is a bug in the Samba server
> and part of my patch trivially fixes this.
> 
> The question I'm pondoring is - is it correct to spoof
> this to show a directory when you are querying with a
> UNIX info level ? On the positive side it hides the
> Samba implementations. On the negative side it makes
> it impossible to write code that actually allows the
> client to see what is really on disk.
> 
> This goes against the spirit of the UNIX extensions,
> which is to show straight "posix" on the wire. Now
> we are in somewhat unknown territory, in that we're
> defining the interface between UNIX extensions and
> MSDFS, but if I spoof to show a directory when it's
> really a symlink I'm lying to clients.

As I understand 'extensions', should not break interface
they are extending. Symlink for DFS junction point is
just the way chosen by samba for storing dfs metadata.
So showing it as directory isn't breaking 'posix' way
at all, just hides particular implementation details.

Also for compatibility with clients it would be better
to behave like MS even with "Unix Ext." turned on.
Working the same way in both modes for similar concepts
reduces headache for everyone.
 
> If I do the spoof there's no way for clients that
> want to see the symlink (maybe in order to change
> it) to do so.

Typically client expects to see directory here.
MS uses another way for getting metadata for such dirs.
When I dig up how they do it, I'll try to write corresponding
code for cifs to show which referrals up/down/connected
for a dfs junction point.


> I could add some "magic foo" and show the symlink
> if the QPATHINFO request flags2 doesn't contain the
> DFS_PATH bit, and show as a directory if it does,
> but this sucks (and I hate magic foo such as this).
> 
> I'm inclined as Volker suggested to leave the current
> behavior with UNIX extensions alone, as the client
> knows it's dealing with a DFS path (after all the
> first call got PATH_NOT_COVERED) and it allows the
> client to code around the discrepancy (as you say
> traverse over DFS junctions now works with the client)
> but still allows the UNIX extensions to show what
> is really on disk in full POSIX glory :-).
> 
> Steve, others, please comment !
> 
> Cheers,
> 
> 	Jeremy.

Magic is really sucks. Because of DFS junction point
is symlink in samba, when unix ext. is on, we had to do
that magic thing in the kernel code.
Now we have following behavior:
1.  when client makes 'ls' for the first time on the 
    directory with DFS links, 'ls' shows them as symlinks.
2.  when we follow through a such 'symlink', it finely
    becomes a directory.
That magic!!!
Of cause it is possible to rewrite such 'symlink' in readdir
syscall handler, but is it the best way? 

IMHO:
  For a client the way better to see a directory from the start.
  (common code for handling this case for MS and Samba servers)
  and no magic at all.

-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Jeremy Allison]

On Wed, May 21, 2008 at 05:57:48PM +0400, Igor Mammedov wrote:
> 
> Magic is really sucks. Because of DFS junction point
> is symlink in samba, when unix ext. is on, we had to do
> that magic thing in the kernel code.
> Now we have following behavior:
> 1.  when client makes 'ls' for the first time on the 
>     directory with DFS links, 'ls' shows them as symlinks.
> 2.  when we follow through a such 'symlink', it finely
>     becomes a directory.
> That magic!!!
> Of cause it is possible to rewrite such 'symlink' in readdir
> syscall handler, but is it the best way? 
> 
> IMHO:
>   For a client the way better to see a directory from the start.
>   (common code for handling this case for MS and Samba servers)
>   and no magic at all.

I can easily do this in the server, the problem is
how we define what is "correct" from the client 
view with the UNIX extensions turned on.

Because we are overloading the filesystem to
store DFS links as symlinks, it's arguable
which way they should be seen. If a symlink
with the corrcet contents is "magically" seen
as a directory, then a client can write a
symlink which is then no longer seen as a
symlink, but turns into a directory when
queried.

The problem is the Samba implementation
is bleeding out onto the wire here. As we're
stuck with it for the time being we have to
make the best of it. Trouble is, I'm not
sure what exactly the right decision is
here..

Jeremy.

Re: dfs path construction fixup for / character in \\server\share component of dfs path

[Steve French]

On Fri, May 23, 2008 at 7:46 PM, Jeremy Allison <jra@samba.org> wrote:
> On Wed, May 21, 2008 at 05:57:48PM +0400, Igor Mammedov wrote:
>>
>> Magic is really sucks. Because of DFS junction point
>> is symlink in samba, when unix ext. is on, we had to do
>> that magic thing in the kernel code.
>> Now we have following behavior:
>> 1.  when client makes 'ls' for the first time on the
>>     directory with DFS links, 'ls' shows them as symlinks.
>> 2.  when we follow through a such 'symlink', it finely
>>     becomes a directory.
>> That magic!!!
>> IMHO:
>>   For a client the way better to see a directory from the start.
>>   (common code for handling this case for MS and Samba servers)
>>   and no magic at all.
>
> I can easily do this in the server, the problem is
> how we define what is "correct" from the client
> view with the UNIX extensions turned on.
>
> The problem is the Samba implementation
> is bleeding out onto the wire here. As we're
> stuck with it for the time being we have to
> make the best of it. Trouble is, I'm not
> sure what exactly the right decision is
> here..

I don't know what is best either ... but whatever we do we should be
careful not to hurt performance ... I would like to know what happens
on: readdir of a directory contains a DFS "symlink"  (followed by
lookup of the symink inode which is cached) followed by get sym link
info.  Does that return STATUS_PATH_NOT_COVERED and everything works
...?

-- 
Thanks,

Steve