Patchwork Can't mount smb shares using mount.cifs with 2.6.31 kernel

login
register
mail settings
Submitter Jeff Layton
Date Oct. 16, 2009, 8:12 p.m.
Message ID <20091016161243.20a52a0c@tlielax.poochiereds.net>
Download mbox | patch
Permalink /patch/36271/
State New
Headers show

Comments

Jeff Layton - Oct. 16, 2009, 8:12 p.m.
On Fri, 16 Oct 2009 14:49:56 -0400
Jeff Layton <jlayton@redhat.com> wrote:

> On Fri, 16 Oct 2009 12:06:59 -0400
> Timothy Normand Miller <theosib@gmail.com> wrote:
> 
> > I'm sending you the captures privately.  The first one is just a
> > mount.  The second one is a mount with an ls.
> > 
> > On Fri, Oct 16, 2009 at 11:43 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > > On Fri, 16 Oct 2009 11:35:07 -0400
> > > Timothy Normand Miller <theosib@gmail.com> wrote:
> > >
> > >> On Fri, Oct 16, 2009 at 10:50 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > >> > On Fri, 16 Oct 2009 10:32:39 -0400
> > >> >
> > >> > A little. Can you try mounting again with "-o noserverino"? Perhaps
> > >> > something is wrong with the inode numbers being generated by the server
> > >> > here...
> > >>
> > >> That fixed the problem.  It mounts properly now.
> > >>
> > >> > If that doesn't help, then maybe also capture some debugging info of
> > >> > you doing an "ls" in the top-level dir after doing the mount.
> > >>
> > >> I can't tell where one set of debug messages end and where the next
> > >> set starts.  Is there something I can do to insert a dummy message
> > >> into dmesg so I can tell where to start copying?
> > >>
> > >> > I doubt this is anything you've done. There were a number of changes in
> > >> > how inodes are handled under CIFS between 2.6.30 and 2.6.31. It's quite
> > >> > possible that one of those changes is causing problems with this server.
> > >>
> > >> Thanks for the help.  Is there anything else I can do to diagnose the
> > >> cause so it can be fixed permanently?
> > >>
> > >>
> > >
> > > Yes, could you send me a wire capture of a mount on the 2.6.31 kernel
> > > without the "-o noserverino" option? I need to have a look at why the
> > > GetSrvInum call isn't doing the right thing here. There are
> > > instructions on the troubleshooting page for doing this.
> > >
> > > Thanks,
> > > --
> > > Jeff Layton <jlayton@redhat.com>
> > >
> > 
> > 
> > 
> 
> Thanks for the info. The mount seems to be successful. The ls makes
> CIFS issue a "FindFile First" which is how SMB does a READDIR on the
> wire. The server sends the response and then after that there is no
> further traffic. I assume that CIFS doesn't like that response for some
> reason, but wireshark unfortunately does not yet implement a dissector
> for the FFF infolevel in use here.
> 
> I'm going to see if I can fix that in wireshark and then may have a
> little more info. What may help in the meantime is the output from
> cifsFYI debugging turned up while doing an "ls" after a fresh mount
> without "-o noserverino".
> 

Yay. Figured out how to fix the wireshark dissector for this FindFile
infolevel. Wireshark patch attached that allows me to dissect these
packets correctly.  I'll plan to send that to the wireshark devs as
soon as I figure out where to send it. The good news is that I think I
see the problem. The bad news is that I'm not quite sure how to fix it
yet and it may even be a server bug.

Prior to 2.6.30, the default was to generate inode numbers out of the
air (noserverino). With 2.6.31, the default is "serverino" which makes
it so that we query the server for inode numbers. When mounting, we do
a QueryPathInfo against the root inode. With serverino enabled, we also
do a FileInternalInfo query against the file for the
"IndexNumber" (which I assumed was also the equivalent of the UniqueId
but maybe isn't?). In any case, that first call succeeds against this
server.

The problem comes in with the FindFile call. There we do a
FIND_FILE_ID_FULL_DIRECTORY_INFO infolevel query. That also succeeds,
but the inode number values in there (FileId's) are zeroed out. That's
technically within the letter of the spec for that call. When the
underlying filesystem doesn't support unique ID's, then it's supposed
to return 0. The problem is that it doesn't make much sense for the
server to claim that it does support unique ID's for one call but not
for others. 

Ideally, there'll be a way to deal with this automatically, but I'll
probably need to ponder this a bit. In any case, thanks for the problem
report. I'll let you know once I come up with something.

Cheers,
Timothy Normand Miller - Nov. 2, 2009, 1:40 a.m.
On Fri, Oct 16, 2009 at 3:12 PM, Jeff Layton <jlayton@redhat.com> wrote:

>
> Yay. Figured out how to fix the wireshark dissector for this FindFile
> infolevel. Wireshark patch attached that allows me to dissect these
> packets correctly.  I'll plan to send that to the wireshark devs as
> soon as I figure out where to send it. The good news is that I think I
> see the problem. The bad news is that I'm not quite sure how to fix it
> yet and it may even be a server bug.
>
> Prior to 2.6.30, the default was to generate inode numbers out of the
> air (noserverino). With 2.6.31, the default is "serverino" which makes
> it so that we query the server for inode numbers. When mounting, we do
> a QueryPathInfo against the root inode. With serverino enabled, we also
> do a FileInternalInfo query against the file for the
> "IndexNumber" (which I assumed was also the equivalent of the UniqueId
> but maybe isn't?). In any case, that first call succeeds against this
> server.
>
> The problem comes in with the FindFile call. There we do a
> FIND_FILE_ID_FULL_DIRECTORY_INFO infolevel query. That also succeeds,
> but the inode number values in there (FileId's) are zeroed out. That's
> technically within the letter of the spec for that call. When the
> underlying filesystem doesn't support unique ID's, then it's supposed
> to return 0. The problem is that it doesn't make much sense for the
> server to claim that it does support unique ID's for one call but not
> for others.
>
> Ideally, there'll be a way to deal with this automatically, but I'll
> probably need to ponder this a bit. In any case, thanks for the problem
> report. I'll let you know once I come up with something.
>

I'm very sorry I took so long to try out this patch.  It appears to
have done the trick.  Thanks!

Patch

From 4f7d1427f79cb26387ee15f5bdec54a3f2bff9fc Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@redhat.com>
Date: Fri, 16 Oct 2009 15:24:31 -0400
Subject: [PATCH] smb dissector: add support for dissecting SEARCH_ID_FULL_DIR_INFO

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 epan/dissectors/packet-smb.c |  129 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 129 insertions(+), 0 deletions(-)

diff --git a/epan/dissectors/packet-smb.c b/epan/dissectors/packet-smb.c
index ec6c26b..98f456a 100644
--- a/epan/dissectors/packet-smb.c
+++ b/epan/dissectors/packet-smb.c
@@ -10036,6 +10036,7 @@  static const value_string ff2_il_vals[] = {
 	{ 0x0102,	"Find File Full Directory Info"},
 	{ 0x0103,	"Find File Names Info"},
 	{ 0x0104,	"Find File Both Directory Info"},
+	{ 0x0105,	"Find File ID Full Directory Info"},
 	{ 0x0202,	"Find File UNIX"},
 	{0, NULL}
 };
@@ -13935,6 +13936,130 @@  dissect_4_3_4_8(tvbuff_t *tvb _U_, packet_info *pinfo _U_,
 	return offset;
 }
 
+
+static int
+dissect_4_3_4_9(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree,
+    int offset, guint16 *bcp, gboolean *trunc)
+{
+	int fn_len, sfn_len;
+	const char *fn, *sfn;
+	int old_offset = offset;
+	proto_item *item = NULL;
+	proto_tree *tree = NULL;
+	smb_info_t *si;
+	guint32 neo;
+	int padcnt;
+
+	si = (smb_info_t *)pinfo->private_data;
+	DISSECTOR_ASSERT(si);
+
+	if(parent_tree){
+		tvb_ensure_bytes_exist(tvb, offset, *bcp);
+		item = proto_tree_add_text(parent_tree, tvb, offset, *bcp, "%s",
+		    val_to_str(si->info_level, ff2_il_vals, "Unknown (0x%02x)"));
+		tree = proto_item_add_subtree(item, ett_smb_ff2_data);
+	}
+
+	/*
+	 * XXX - I have not seen any of these that contain a resume
+	 * key, even though some of the requests had the "return resume
+	 * key" flag set.
+	 */
+
+	/* next entry offset */
+	CHECK_BYTE_COUNT_SUBR(4);
+	neo = tvb_get_letohl(tvb, offset);
+	proto_tree_add_uint(tree, hf_smb_next_entry_offset, tvb, offset, 4, neo);
+	COUNT_BYTES_SUBR(4);
+
+	/* file index */
+	CHECK_BYTE_COUNT_SUBR(4);
+	proto_tree_add_item(tree, hf_smb_file_index, tvb, offset, 4, TRUE);
+	COUNT_BYTES_SUBR(4);
+
+        /* dissect standard 8-byte timestamps */
+	offset = dissect_smb_standard_8byte_timestamps(tvb, pinfo, tree, offset, bcp, trunc);
+	if (*trunc) {
+	  return offset;
+	}
+
+	/* end of file */
+	CHECK_BYTE_COUNT_SUBR(8);
+	proto_tree_add_item(tree, hf_smb_end_of_file, tvb, offset, 8, TRUE);
+	COUNT_BYTES_SUBR(8);
+
+	/* allocation size */
+	CHECK_BYTE_COUNT_SUBR(8);
+	proto_tree_add_item(tree, hf_smb_alloc_size64, tvb, offset, 8, TRUE);
+	COUNT_BYTES_SUBR(8);
+
+	/* Extended File Attributes */
+	CHECK_BYTE_COUNT_SUBR(4);
+	offset = dissect_file_ext_attr(tvb, tree, offset);
+	*bcp -= 4;
+
+	/* file name len */
+	CHECK_BYTE_COUNT_SUBR(4);
+	fn_len = tvb_get_letohl(tvb, offset);
+	proto_tree_add_uint(tree, hf_smb_file_name_len, tvb, offset, 4, fn_len);
+	COUNT_BYTES_SUBR(4);
+
+	/*
+	 * EA length.
+	 *
+	 * XXX - in one captures, this has the topmost bit set, and the
+	 * rest of the bits have the value 7.  Is the topmost bit being
+	 * set some indication that the value *isn't* the length of
+	 * the EAs?
+	 */
+	CHECK_BYTE_COUNT_SUBR(4);
+	proto_tree_add_item(tree, hf_smb_ea_list_length, tvb, offset, 4, TRUE);
+	COUNT_BYTES_SUBR(4);
+
+	/* reserved word */
+	CHECK_BYTE_COUNT_SUBR(4);
+	proto_tree_add_item(tree, hf_smb_reserved, tvb, offset, 4, TRUE);
+	COUNT_BYTES_SUBR(4);
+
+	/* UniqueID */
+	CHECK_BYTE_COUNT_SUBR(8);
+	proto_tree_add_item(tree, hf_smb_index_number, tvb, offset, 8, TRUE);
+	COUNT_BYTES_SUBR(8);
+
+	/* file name */
+	fn = get_unicode_or_ascii_string(tvb, &offset, si->unicode, &fn_len, FALSE, TRUE, bcp);
+	CHECK_STRING_SUBR(fn);
+	proto_tree_add_string(tree, hf_smb_file_name, tvb, offset, fn_len,
+		fn);
+	COUNT_BYTES_SUBR(fn_len);
+
+	if (check_col(pinfo->cinfo, COL_INFO)) {
+		col_append_fstr(pinfo->cinfo, COL_INFO, " %s",
+		    format_text(fn, strlen(fn)));
+	}
+
+	/* skip to next structure */
+	if(neo){
+		padcnt = (old_offset + neo) - offset;
+		if (padcnt < 0) {
+			/*
+			 * XXX - this is bogus; flag it?
+			 */
+			padcnt = 0;
+		}
+		if (padcnt != 0) {
+			CHECK_BYTE_COUNT_SUBR(padcnt);
+			COUNT_BYTES_SUBR(padcnt);
+		}
+	}
+
+	proto_item_append_text(item, " File: %s", format_text(fn, strlen(fn)));
+	proto_item_set_len(item, offset-old_offset);
+
+	*trunc = FALSE;
+	return offset;
+}
+
 /*dissect the data block for TRANS2_FIND_FIRST2*/
 static int
 dissect_ff2_response_data(tvbuff_t * tvb, packet_info * pinfo,
@@ -13979,6 +14104,10 @@  dissect_ff2_response_data(tvbuff_t * tvb, packet_info * pinfo,
 		offset = dissect_4_3_4_6(tvb, pinfo, tree, offset, bcp,
 		    trunc);
 		break;
+	case 0x0105:	/*Find File ID Full Directory Info */
+		offset = dissect_4_3_4_9(tvb, pinfo, tree, offset, bcp,
+		    trunc);
+		break;
 	case 0x0202:	/*Find File UNIX*/
 		offset = dissect_4_3_4_8(tvb, pinfo, tree, offset, bcp,
 		    trunc);
-- 
1.6.2.5