Preliminary patch to 2.0.33 fixing smbfs NT4 mtime bug

Ulrik Dickow (ukd@kampsax.dk)
19 Jan 1998 22:36:45 +0100


Summary:

The smbfs in kernel 2.0.31-33 causes serious corruption of file
modification times of files on NT 4.0 (SP1/SP3) workstations & servers:

1) Opening a file for reading via smbfs (and closing it) corrupts the
mtime as seen locally on NT!

2) Listing (stat'ing) a _file_ via smbfs returns a bogus mtime,
but does not propagate it.

3) Listing a _directory_ (with e.g. ls(1)) returns the correct NT
mtimes. Of course, if Linux have previously corrupted the NT file,
NT will dutifully return the bogus mtime in the new listing too.

Windows NT 3.51 (SP0/SP5) is not affected. Don't know about Win95.

A preliminary patch at the end of this mail fixes the problem, provided
EXTRA_CFLAGS='-DCONFIG_SMB_NT4'
is given on the `make' or `make modules' command line.
Win95 testers wanted!

On Friday 16 Jan 1998, Eric Hoeltzel <eric@dogbert.sitewerks.com> said:

> I'm getting some weird dates with smbfs mounting some NT4S shares.
> Smbmount is version 2.0.2, kernel is 2.0.32. Any clues?
>[...]
> -rwxr-xr-x 1 danny users 21149 Thu Sep 11 04:52:16 1941 x
> -rwxr-xr-x 1 danny users 32 Tue Jan 9 10:27:12 1962 xx
>
> The last two files, x and xx, I created with a find > x. Immediately
> after creating them I checked the date and it was correct.
> A bit later they had changed for no apparent reason.
>
> Any ideas?

Certainly. Shortly before you sent your mail, I captured a few small test
sessions with ls(1) and cat(1) on directories/files smbmounted on Linux
2.0.33 from NT4 SP3, and from NT3.51 SP5. This resulted in the conclusions
summarized above. That NT 4.0 SP1 is affected too, and that NT3.51 SP0 is
not, comes from earlier experience.

So 2.0.33's smb_proc_getattr_core() doesn't work with NT4. What can we do?
I see several possibilities:

1) Reviving the smb_proc_getattr_trans2 from Linux 2.0.30
that sends a TRANSACT2_QPATHINFO command at info level 1.
However, this command is known to be awfully slow with Win95.
The 2.0.30 smbfs was slow against NT 3.51 too, but perhaps for other
reasons.

2) Trying to backport the info level 259/260 code from 2.1.79, since
Bill Hawes said it `works pretty well with NT4'.
Again Win95 seems to loose.

3) Backporting Bill Hawes' smb_proc_getattr_ff() ("updated patch for 2.1.79
smbfs", 15 Jan 1998). It uses a info level 1 TRANSACT2_FINDFIRST
command to get the file attributes, like 2.0.33's smb_proc_readdir_long
does.
This solution was meant to give hope to Win95 lusers.

I chose solution 3), i.e. building on pieces partly from Bill Hawes' code,
partly from the 2.0.33 smb_proc_readdir_long() and smb_decode_long_dirent().

Apart from debugging code, all new code is currently wrapped in `#ifdef
CONFIG_SMB_NT4' ... `#endif' pairs, analogous to the existing
CONFIG_SMB_WIN95 option. The patch is preliminary. The final patch depends
on how well the preliminary one is received among testers and developers:

* If all non-NT4 folks are happy with it, the #ifdef's can go away.
For example, a `find . -ls' on the D drive of my NT 3.51 SP5 machine
completes in 52 seconds after the patch, compared with 42 before
(7000 files, 400 MB). That factor 1.23 slowdown is insignificant to me.

* If some think a choice is important for performance reasons, the
#ifdef's stay, and then appropriate changes to fs/Config.in and
Documentation/Configure.help must be added, with a suitable default.

* If Win95 (or other) testers find that the patch works very poorly,
then somebody else will come up with a completely different fix.

Finally, remember that this preliminary patch requires you to add a flag to
the make(1) command line (executed from /usr/src/linux/), e.g.

make EXTRA_CFLAGS=-DCONFIG_SMB_NT4 # or
make EXTRA_CFLAGS=-DCONFIG_SMB_NT4 modules # (preferred)

As usual for 2.0.3x smbfs, debugging information is obtained by adding
`-DDEBUG_SMB=1' or `-DDEBUG_SMB=2' to the EXTRA_CFLAGS. The latter is
extremely verbose. (BTW, the patch preliminarily excludes the password from
being logged. The final patch will probably exclude the last hunk, turning
password logging back on, even though I don't like it.)

====== Cut here ===========================================================
--- linux-2.0.33/fs/smbfs/proc.c.shipped Mon Oct 27 12:30:47 1997
+++ linux-2.0.33/fs/smbfs/proc.c Mon Jan 19 18:19:01 1998
@@ -1356,7 +1356,7 @@

smb_lock_server(server);

- DDPRINTK("smb_proc_getattr: %s\n", name);
+ DDPRINTK("smb_proc_getattr_core: name=%s\n", name);

retry:
buf = server->packet;
@@ -1378,11 +1378,100 @@
entry->f_ctime = entry->f_atime =
entry->f_mtime = local2utc(DVAL(buf, smb_vwv1));

+ DDPRINTK("smb_proc_getattr_core: mtime=%ld\n", entry->f_mtime);
+
entry->f_size = DVAL(buf, smb_vwv3);
smb_unlock_server(server);
return 0;
}

+#ifdef CONFIG_SMB_NT4
+/*
+ * This version use the trans2 findfirst to get the attribute info.
+ */
+static int
+smb_proc_getattr_ff(struct inode *dir, const char *name, int len,
+ struct smb_dirent *entry)
+{
+ unsigned char *resp_data = NULL;
+ unsigned char *resp_param = NULL;
+ int resp_data_len = 0;
+ int resp_param_len = 0;
+
+ char param[SMB_MAXPATHLEN + 1 + 12];
+ int mask_len;
+ unsigned char *mask = &(param[12]);
+
+ int result;
+ char *p;
+ struct smb_server *server = SMB_SERVER(dir);
+
+ mask_len = smb_encode_path(server, mask,
+ SMB_INOP(dir), name, len) - mask;
+
+ mask[mask_len] = 0;
+
+ DDPRINTK("smb_proc_getattr_ff: mask=%s\n", mask);
+
+ smb_lock_server(server);
+
+ retry:
+
+ WSET(param, 0, aSYSTEM | aHIDDEN | aDIR);
+ WSET(param, 2, 1); /* max count */
+ WSET(param, 4, 2 + 1); /* close on end + close after this call */
+ WSET(param, 6, 1); /* info level */
+ DSET(param, 8, 0);
+
+ result = smb_trans2_request(server, TRANSACT2_FINDFIRST,
+ 0, NULL, 12 + mask_len + 1, param,
+ &resp_data_len, &resp_data,
+ &resp_param_len, &resp_param);
+
+ if (result < 0)
+ {
+ if (smb_retry(server))
+ {
+ DPRINTK("smb_proc_getattr_ff: error=%d, retrying\n",
+ result);
+ goto retry;
+ }
+ goto out;
+ }
+ if (server->rcls != 0)
+ {
+ result = -smb_errno(server->rcls, server->err);
+ if (result != -ENOENT)
+ DPRINTK("smb_proc_getattr_ff: rcls=%d, err=%d\n",
+ server->rcls, server->err);
+ goto out;
+ }
+ /* Make sure we got enough data ... */
+ result = -EINVAL; /* WVAL(resp_param, 2) is ff_searchcount */
+ if (resp_data_len < 22 || WVAL(resp_param, 2) != 1)
+ {
+ DPRINTK("smb_proc_getattr_ff: bad result, len=%d, count=%d\n",
+ resp_data_len, WVAL(resp_param, 2));
+ goto out;
+ }
+ /* Decode the response (info level 1, as in smb_decode_long_dirent) */
+ p = resp_data;
+ entry->f_ctime = date_dos2unix(WVAL(p, 2), WVAL(p, 0));
+ entry->f_atime = date_dos2unix(WVAL(p, 6), WVAL(p, 4));
+ entry->f_mtime = date_dos2unix(WVAL(p, 10), WVAL(p, 8));
+ entry->f_size = DVAL(p, 12);
+ entry->attr = WVAL(p, 20);
+
+ DDPRINTK("smb_proc_getattr_ff: attr=%x\n", entry->attr);
+
+ result = 0;
+
+out:
+ smb_unlock_server(server);
+ return result;
+}
+#endif /* CONFIG_SMB_NT4 */
+
int
smb_proc_getattr(struct inode *dir, const char *name, int len,
struct smb_dirent *entry)
@@ -1391,7 +1480,14 @@
int result;

smb_init_dirent(server, entry);
- result = smb_proc_getattr_core(dir, name, len, entry);
+
+#ifdef CONFIG_SMB_NT4
+ if (server->protocol >= PROTOCOL_LANMAN2)
+ result = smb_proc_getattr_ff(dir, name, len, entry);
+ else
+#endif
+ result = smb_proc_getattr_core(dir, name, len, entry);
+
smb_finish_dirent(server, entry);

entry->len = len;
@@ -1612,8 +1708,8 @@
word passlen = strlen(server->m.password);
word userlen = strlen(server->m.username);

- DPRINTK("smb_proc_connect: password = %s\n",
- server->m.password);
+/* DPRINTK("smb_proc_connect: password = %s\n", */
+/* server->m.password); */
DPRINTK("smb_proc_connect: usernam = %s\n",
server->m.username);
DPRINTK("smb_proc_connect: blkmode = %d\n",
====== Cut here ===========================================================

Happy testing

-- 
Ulrik Dickow, Systems Programmer           Snail Mail: Kampsax Technology
E-mail: ukd@kampsax.dk                                 P.O. Box 1142
Phone:  +45 36 39 08 00                                DK-2650 Hvidovre
Fax:    +45 36 77 03 01                                Denmark