smbfs: patch for directory listing

[Urban Widmark]

Hello everyone

I have recently reported problems reading a certain directory from an NT
server on this list (and speculated in causes). I have a few workarounds
and now I'm looking for some feedback on what the proper fix is and for
people to test if my patch works for them.

Please test smbfs with and without the patch on the directory in this
zipfile (all files are empty):
http://www.cs.umu.se/~urbanw/samba/samba-dir.zip
(you should probably not unpack it through smbfs)

I'd like to know if it works or not (before and after), and what the
server is running. I have tested NT4 SP3 & SP4 (works after patching) and
samba-2.0.4 (works both before and after). Other directories should of
course still work.


What happens is that when listing the dir the NT4 server returns an error
on the first find_next call:
smb_proc_readdir_long: name=, entries=117, rcls=1, err=123
and the dir can't be listed. A samba-2.0.4 server works fine without the
patch, so this is possibly a NT bug (or samba not being equally
picky/not using the same mechanism for resuming a dir listing).


I have found the following workarounds for reading this dir:
* decrease the max number of filenames returned to something below the 119
  the server wants to return on the first call.
* increase or decrease the buffer size (which is sent to the server as a
  max_xmit value)
* remove the request for resume keys (which don't seem to be used anyway)
  for both FIND_FIRST and FIND_NEXT

This part of smbfs looks very much like libsmb, and yet smbclient works
fine (with the resume key requests). A difference is the infolevel,
smbclient uses 260 but smbfs uses 259, another is the much larger
buffersize of smbclient (0xffff vs 4096-17).


Removing the flags for resume keys stops smbfs from working with a
samba-2.0.4 server (and possibly others). That is probably because of a
bug (?) in smbfs that it does not return the "lastname" properly.

The "LastNameOffset" points to the last record in the data area, not to
the actual filename. But the code simply does
	strncpy(mask, lastname, mask_len);
where lastname is a pointer to a record looking something like this:
00000000 00000000 14000000 45646974426f7844656661756c742e6a61766100
nnnnnnnn iiiiiiii llllllll
n - next record
i - file index
l - filename length

As I understand it the mask should be the string, and not the record.
smbclient (libsmb) does the same thing, so it is possibly also wrong.


Hacking samba-2.0.6pre1 libsmb/clientgen.c to use infolevel 259 causes the
same problems when also changing the buffersize to 4096-17. Removing
resume key makes it work with NT, but samba-2.0.4 still complains (and I
can't get it to work by simply adding 12, there is something else wrong as
well).


One reason for why it would be correct to remove the resume key flags is
that no resume keys are sent (as I read the draft-spec anyway, and as some
has said on the CIFS mailinglist).

I do not know where 4096-17 comes from, but the smb header is much larger
than that. Maybe the correct fix is simply to change the size of the
buffer. But maybe changing the buffersize simply makes another directory
magically not work.


The attached patch is for Linux 2.2.12, but 2.2.13 and 2.3.x should be
similar.

/Urban
-------------- next part --------------
diff -ur linux-2.2.12-clean/fs/smbfs/proc.c linux/fs/smbfs/proc.c
--- linux-2.2.12-clean/fs/smbfs/proc.c	Sat Oct 23 17:44:54 1999
+++ linux/fs/smbfs/proc.c	Sat Oct 23 22:25:49 1999
@@ -1599,8 +1599,7 @@
 			command = TRANSACT2_FINDFIRST;
 			WSET(param, 0, aSYSTEM | aHIDDEN | aDIR);
 			WSET(param, 2, max_matches);	/* max count */
-			WSET(param, 4, 8 + 4 + 2);	/* resume required +
-							   close on end +
+			WSET(param, 4, 4 + 2);		/* close on end +
 							   continue */
 			WSET(param, 6, info_level);
 			DSET(param, 8, 0);
@@ -1615,8 +1614,7 @@
 			WSET(param, 2, max_matches);	/* max count */
 			WSET(param, 4, info_level);
 			DSET(param, 6, ff_resume_key);	/* ff_resume_key */
-			WSET(param, 10, 8 + 4 + 2);	/* resume required +
-							   close on end +
+			WSET(param, 10, 4 + 2);		/* close on end +
 							   continue */
 			if (server->mnt->version & SMB_FIX_WIN95)
 			{
@@ -1687,6 +1685,12 @@
 			case 259:
  				if (ff_lastname < resp_data_len)
 					mask_len = resp_data_len - ff_lastname;
+				/* The lastname pointer points to the record,
+				   not to the name. */
+				lastname += 12;
+				mask_len -= 12;
+				if(mask_len < 0)
+					mask_len = 0;
 				break;
 			case 1:
 				/* Win NT 4.0 doesn't set the length byte */