This is a *big* checkin that may break some things, but implements the

new open mechanism Andrew & I discussed.

config.sub:
configure: Included the QNX patch.

include/vfs.h:
smbd/vfs-wrap.c:
smbd/vfs.c: Added ftruncate vfs call (needed).

Note that we will also need locking calls in the vfs (to be added).

lib/util_unistr.c:
nmbd/nmbd_processlogon.c: Fix for NT domain logons causing nmbd to core dump.
                          Also fix for sidsize DOS bug.

locking/locking.c: Check value of ret before using it for memdup.

printing/printing.c: Convert print_fsp_open to return an allocated fsp.

rpc_server/srv_lsa.c: Fix for NT domain logons.

I have removed all use of lp_share_modes() from the code (although I
left the parameter in the table for backwards compatibility). It no longer makes
sense for this to exist.

smbd/close.c: Removed lp_share_modes().
smbd/fileio.c: Fixed parameters to unlock_share_entry call in panic code.
smbd/files.c: Correctly set the unix_ERR_code to ERRnofids on fsp allocation fail.

smbd/nttrans.c:
smbd/reply.c:
smbd/trans2.c: Changed all occurrences of open_file_shared/open_directory/
               open_file_stat to return an fsp from the call.

smbd/open.c: Changed all occurrences of open_file_shared/open_directory/
             open_file_stat to return an fsp from the call.

In addition I have fixed a long standing race condition in the deny mode
processing w.r.t. two smbd's creating a file. Andrew, please note that
your original idea of using open with O_EXCL in this case would not work
(I went over the races very carefully) and so we must re-check deny modes
*after* the open() call returns. This is because there is a race between
the open with O_EXCL and the lock of the share mode entry. Imagine the
case where the first smbd does the open with O_EXCL and a deny mode of DENY_ALL,
but is pre-empted before it locks the share modes and creates the deny
mode entry for DENY_ALL. A second smbd could then come in with O_RDONLY
and a deny mode of DENY_NONE and the two opens would be allowed.

The *only* way to fix this race is to lock the share modes after the
open and then do the deny mode checks *after* this lock in the case
where the file did not originally exist.

This code will need extensive testing but seems to initially work.

Jeremy.
(This used to be commit ab0ecc39d688f16b9692fe90b991f0b89287070a)

1 files changed, 78 insertions, 87 deletions

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 2ca06ab746..f0a645d6c7 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -221,10 +221,6 @@ static int call_trans2open(connection_struct *conn, char *inbuf, char *outbuf,
 
   unix_convert(fname,conn,0,&bad_path,NULL);
     
-  fsp = file_new();
-  if (!fsp)
-    return(ERROR(ERRSRV,ERRnofids));
-
   if (!check_name(fname,conn))
   {
     if((errno == ENOENT) && bad_path)
@@ -232,23 +228,21 @@ static int call_trans2open(connection_struct *conn, char *inbuf, char *outbuf,
       unix_ERR_class = ERRDOS;
       unix_ERR_code = ERRbadpath;
     }
-    file_free(fsp);
     return(UNIXERROR(ERRDOS,ERRnoaccess));
   }
 
   unixmode = unix_mode(conn,open_attr | aARCH, fname);
       
-  open_file_shared(fsp,conn,fname,open_mode,open_ofun,unixmode,
+  fsp = open_file_shared(conn,fname,open_mode,open_ofun,unixmode,
 		   oplock_request, &rmode,&smb_action);
       
-  if (!fsp->open)
+  if (!fsp)
   {
     if((errno == ENOENT) && bad_path)
     {
       unix_ERR_class = ERRDOS;
       unix_ERR_code = ERRbadpath;
     }
-    file_free(fsp);
     return(UNIXERROR(ERRDOS,ERRnoaccess));
   }
 
@@ -1794,113 +1788,110 @@ static int call_trans2setfilepathinfo(connection_struct *conn,
            * the share mode contained ALLOW_SHARE_DELETE
            */
 
-          if(lp_share_modes(SNUM(conn)))
+          if(!GET_ALLOW_SHARE_DELETE(fsp->share_mode))
+            return(ERROR(ERRDOS,ERRnoaccess));
+
+          /*
+           * If the flag has been set then
+           * modify the share mode entry for all files we have open
+           * on this device and inode to tell other smbds we have 
+           * changed the delete on close flag.
+           */
+
+          if(delete_on_close && !GET_DELETE_ON_CLOSE_FLAG(fsp->share_mode))
           {
-            if(!GET_ALLOW_SHARE_DELETE(fsp->share_mode))
+            int i;
+            files_struct *iterate_fsp;
+            SMB_DEV_T dev = fsp->dev;
+            SMB_INO_T inode = fsp->inode;
+            int num_share_modes;
+            share_mode_entry *current_shares = NULL;
+
+            if (lock_share_entry_fsp(fsp) == False)
               return(ERROR(ERRDOS,ERRnoaccess));
 
             /*
-             * If the flag has been set then
-             * modify the share mode entry for all files we have open
-             * on this device and inode to tell other smbds we have 
-             * changed the delete on close flag.
+             * Before we allow this we need to ensure that all current opens
+             * on the file have the GET_ALLOW_SHARE_DELETE flag set. If they
+             * do not then we deny this (as we are essentially deleting the
+             * file at this point.
              */
 
-            if(delete_on_close && !GET_DELETE_ON_CLOSE_FLAG(fsp->share_mode))
+            num_share_modes = get_share_modes(conn, dev, inode, &current_shares);
+            for(i = 0; i < num_share_modes; i++)
             {
-              int i;
-              files_struct *iterate_fsp;
-              SMB_DEV_T dev = fsp->dev;
-              SMB_INO_T inode = fsp->inode;
-              int num_share_modes;
-              share_mode_entry *current_shares = NULL;
-
-              if (lock_share_entry_fsp(fsp) == False)
-                return(ERROR(ERRDOS,ERRnoaccess));
-
-              /*
-               * Before we allow this we need to ensure that all current opens
-               * on the file have the GET_ALLOW_SHARE_DELETE flag set. If they
-               * do not then we deny this (as we are essentially deleting the
-               * file at this point.
-               */
-
-              num_share_modes = get_share_modes(conn, dev, inode, &current_shares);
-              for(i = 0; i < num_share_modes; i++)
+              if(!GET_ALLOW_SHARE_DELETE(current_shares[i].share_mode))
               {
-                if(!GET_ALLOW_SHARE_DELETE(current_shares[i].share_mode))
-                {
-                  DEBUG(5,("call_trans2setfilepathinfo: refusing to set delete on close flag for fnum = %d, \
+                DEBUG(5,("call_trans2setfilepathinfo: refusing to set delete on close flag for fnum = %d, \
 file %s as a share exists that was not opened with FILE_DELETE access.\n",
-                        fsp->fnum, fsp->fsp_name ));
-                  /*
-                   * Release the lock.
-                   */
+                      fsp->fnum, fsp->fsp_name ));
+                /*
+                 * Release the lock.
+                 */
 
-                  unlock_share_entry_fsp(fsp);
+                unlock_share_entry_fsp(fsp);
 
-                  /*
-                   * current_shares was malloced by get_share_modes - free it here.
-                   */
+                /*
+                 * current_shares was malloced by get_share_modes - free it here.
+                 */
 
-                  free((char *)current_shares);
+                free((char *)current_shares);
 
-                  /*
-                   * Even though share violation would be more appropriate here,
-                   * return ERRnoaccess as that's what NT does.
-                   */
+                /*
+                 * Even though share violation would be more appropriate here,
+                 * return ERRnoaccess as that's what NT does.
+                 */
 
-                  return(ERROR(ERRDOS,ERRnoaccess));
-                }
+                return(ERROR(ERRDOS,ERRnoaccess));
               }
+            }
 
-              /*
-               * current_shares was malloced by get_share_modes - free it here.
-               */
+            /*
+             * current_shares was malloced by get_share_modes - free it here.
+             */
 
-              free((char *)current_shares);
+            free((char *)current_shares);
 
-              DEBUG(10,("call_trans2setfilepathinfo: %s delete on close flag for fnum = %d, file %s\n",
-                   delete_on_close ? "Adding" : "Removing", fsp->fnum, fsp->fsp_name ));
+            DEBUG(10,("call_trans2setfilepathinfo: %s delete on close flag for fnum = %d, file %s\n",
+                 delete_on_close ? "Adding" : "Removing", fsp->fnum, fsp->fsp_name ));
 
-              /*
-               * Go through all files we have open on the same device and
-               * inode (hanging off the same hash bucket) and set the DELETE_ON_CLOSE_FLAG.
-               * Other smbd's that have this file open will have to fend for themselves. We
-               * take care of this (rare) case in close_file(). See the comment there.
-               */
+            /*
+             * Go through all files we have open on the same device and
+             * inode (hanging off the same hash bucket) and set the DELETE_ON_CLOSE_FLAG.
+             * Other smbd's that have this file open will have to fend for themselves. We
+             * take care of this (rare) case in close_file(). See the comment there.
+             */
 
-              for(iterate_fsp = file_find_di_first(dev, inode); iterate_fsp;
-                                    iterate_fsp = file_find_di_next(iterate_fsp))
-              {
-                int new_share_mode = (delete_on_close ? 
-                                      (iterate_fsp->share_mode | DELETE_ON_CLOSE_FLAG) :
-                                      (iterate_fsp->share_mode & ~DELETE_ON_CLOSE_FLAG) );
+            for(iterate_fsp = file_find_di_first(dev, inode); iterate_fsp;
+                                  iterate_fsp = file_find_di_next(iterate_fsp))
+            {
+              int new_share_mode = (delete_on_close ? 
+                                    (iterate_fsp->share_mode | DELETE_ON_CLOSE_FLAG) :
+                                    (iterate_fsp->share_mode & ~DELETE_ON_CLOSE_FLAG) );
 
-                DEBUG(10,("call_trans2setfilepathinfo: Changing share mode for fnum %d, file %s \
+              DEBUG(10,("call_trans2setfilepathinfo: Changing share mode for fnum %d, file %s \
 dev = %x, inode = %.0f from %x to %x\n", 
-                      iterate_fsp->fnum, iterate_fsp->fsp_name, (unsigned int)dev, 
-                      (double)inode, iterate_fsp->share_mode, new_share_mode ));
+                    iterate_fsp->fnum, iterate_fsp->fsp_name, (unsigned int)dev, 
+                    (double)inode, iterate_fsp->share_mode, new_share_mode ));
 
-                if(modify_share_mode(iterate_fsp, new_share_mode, iterate_fsp->oplock_type)==False)
-                  DEBUG(0,("call_trans2setfilepathinfo: failed to change delete on close for fnum %d, \
+              if(modify_share_mode(iterate_fsp, new_share_mode, iterate_fsp->oplock_type)==False)
+                DEBUG(0,("call_trans2setfilepathinfo: failed to change delete on close for fnum %d, \
 dev = %x, inode = %.0f\n", iterate_fsp->fnum, (unsigned int)dev, (double)inode));
-              }
+            }
 
-              /*
-               * Set the delete on close flag in the reference
-               * counted struct. Delete when the last reference
-               * goes away.
-               */
-             fsp->delete_on_close = delete_on_close;
+            /*
+             * Set the delete on close flag in the reference
+             * counted struct. Delete when the last reference
+             * goes away.
+             */
+           fsp->delete_on_close = delete_on_close;
 
-             unlock_share_entry_fsp(fsp);
+           unlock_share_entry_fsp(fsp);
 
-             DEBUG(10, ("call_trans2setfilepathinfo: %s delete on close flag for fnum = %d, file %s\n",
-                   delete_on_close ? "Added" : "Removed", fsp->fnum, fsp->fsp_name ));
+           DEBUG(10, ("call_trans2setfilepathinfo: %s delete on close flag for fnum = %d, file %s\n",
+                 delete_on_close ? "Added" : "Removed", fsp->fnum, fsp->fsp_name ));
 
-            } /* end if(delete_on_close && !GET_DELETE_ON_CLOSE_FLAG(fsp->share_mode)) */
-          } /* end if lp_share_modes() */
+          } /* end if(delete_on_close && !GET_DELETE_ON_CLOSE_FLAG(fsp->share_mode)) */
         } /* end if is_directory. */
       } else
         return(ERROR(ERRDOS,ERRunknownlevel));