File deny mode still wrong :-)

[Jeremy Allison]

After all these years...... we still have it wrong !

Check out the following bug report :

> If a file is resident on NT and the first user opens it read/write with DENY_READ then a subsequent
> attempt by a second user (running under Windows 95) to open it read/write DENY_NONE fails.
> 
> Under samba 2.0.5a the second open succeeds but the file is write only.

I've checked this against NT4.0 SP5 and it is true. Samba does
this differently compared to NT (under 2.0.6 and pre-2.0.7 also).

The reason is this code in smbd/open.c:check_share_mode() :

    if ((access_allowed == AFAIL) ||
        (!fcbopen && (access_allowed == AREAD && *flags == O_RDWR)) ||
        (access_allowed == AREAD && *flags == O_WRONLY) ||
        (access_allowed == AWRITE && *flags == O_RDONLY))

What we do is only fail if the new r/w open mode *explicitly*
disagrees with the DENY mode, and if it does not, we allow
the file open but change the requested r/w open mode to one
that is compatible with the existing deny modes (the lines :

    if (access_allowed == AREAD)
      *flags = O_RDONLY;

    if (access_allowed == AWRITE)
      *flags = O_WRONLY;

below).

Looking at the X/Open spec deny mode table, page 19, this
*seems* to be the right thing to do - however, if you look
at this carefully, the values returned are the file access
r/w modes that *would be allowed* - not the file access modes
that you should force the incoming open to match !

The correct code should be (here's the patch - also changing
the open modes to symbolic constants rather than numbers) :

Sigh. I *HATE* SMB :-) :-).

If someone could test this out in a wider test setting than
I have access to I'd appreciate it.

Jeremy.

Index: smbd/open.c
===================================================================
RCS file: /data/cvs/samba/source/smbd/open.c,v
retrieving revision 1.26.2.28
diff -u -r1.26.2.28 open.c
--- open.c	1999/12/06 20:54:03	1.26.2.28
+++ open.c	2000/01/08 01:17:15
@@ -631,7 +631,7 @@
     if (old_deny == new_deny && share_pid == pid) 
 	return(AALL);    
 
-    if (old_mode == 0) return(AREAD);
+    if (old_mode == DOS_OPEN_RDONLY) return(AREAD);
 
     /* the new smbpub.zip spec says that if the file extension is
        .com, .dll, .exe or .sym then allow the open. I will force
@@ -651,14 +651,14 @@
   switch (new_deny) 
     {
     case DENY_WRITE:
-      if (old_deny==DENY_WRITE && old_mode==0) return(AREAD);
-      if (old_deny==DENY_READ && old_mode==0) return(AWRITE);
-      if (old_deny==DENY_NONE && old_mode==0) return(AALL);
+      if (old_deny==DENY_WRITE && old_mode==DOS_OPEN_RDONLY) return(AREAD);
+      if (old_deny==DENY_READ && old_mode==DOS_OPEN_RDONLY) return(AWRITE);
+      if (old_deny==DENY_NONE && old_mode==DOS_OPEN_RDONLY) return(AALL);
       return(AFAIL);
     case DENY_READ:
-      if (old_deny==DENY_WRITE && old_mode==1) return(AREAD);
-      if (old_deny==DENY_READ && old_mode==1) return(AWRITE);
-      if (old_deny==DENY_NONE && old_mode==1) return(AALL);
+      if (old_deny==DENY_WRITE && old_mode==DOS_OPEN_WRONLY) return(AREAD);
+      if (old_deny==DENY_READ && old_mode==DOS_OPEN_WRONLY) return(AWRITE);
+      if (old_deny==DENY_NONE && old_mode==DOS_OPEN_WRONLY) return(AALL);
       return(AFAIL);
     case DENY_NONE:
       if (old_deny==DENY_WRITE) return(AREAD);
@@ -711,8 +711,8 @@
 
     if ((access_allowed == AFAIL) ||
         (!fcbopen && (access_allowed == AREAD && *flags == O_RDWR)) ||
-        (access_allowed == AREAD && *flags == O_WRONLY) ||
-        (access_allowed == AWRITE && *flags == O_RDONLY))
+        (access_allowed == AREAD && *flags != O_RDONLY) ||
+        (access_allowed == AWRITE && *flags != O_WRONLY))
     {
       DEBUG(2,("Share violation on file (%d,%d,%d,%d,%s,fcbopen = %d, flags = %d) = %d\n",
                 deny_mode,old_deny_mode,old_open_mode,



-- 
--------------------------------------------------------
Buying an operating system without source is like buying
a self-assembly Space Shuttle with no instructions.
--------------------------------------------------------