Author: jra
Date: 2006-05-17 23:15:53 +0000 (Wed, 17 May 2006)
New Revision: 15668

WebSVN: 
http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=15668

Log:
DOS or FCB opens share one share mode entry from different
fsp pointers. Ensure we cope with this to pass Samba4
DENY tests (we used to pass these, there must have been
a regression with newer code). We now pass them.
Jeremy

Modified:
   branches/SAMBA_3_0/source/include/smb.h
   branches/SAMBA_3_0/source/locking/locking.c
   branches/SAMBA_3_0/source/smbd/close.c
   branches/SAMBA_3_0/source/smbd/files.c
   branches/SAMBA_3_0/source/smbd/oplock.c
   branches/SAMBA_3_0/source/smbd/oplock_irix.c
   branches/SAMBA_3_0/source/smbd/oplock_linux.c


Changeset:
Modified: branches/SAMBA_3_0/source/include/smb.h
===================================================================
--- branches/SAMBA_3_0/source/include/smb.h     2006-05-17 22:21:24 UTC (rev 
15667)
+++ branches/SAMBA_3_0/source/include/smb.h     2006-05-17 23:15:53 UTC (rev 
15668)
@@ -404,6 +404,7 @@
                                 * DELETE_ON_CLOSE is not stored in the share
                                 * mode database.
                                 */
+       unsigned long file_id;
 };
 
 struct timed_event;
@@ -438,7 +439,6 @@
        struct share_mode_entry *pending_break_messages;
        int num_pending_break_messages;
 
-       unsigned long file_id;
        BOOL can_lock;
        BOOL can_read;
        BOOL can_write;

Modified: branches/SAMBA_3_0/source/locking/locking.c
===================================================================
--- branches/SAMBA_3_0/source/locking/locking.c 2006-05-17 22:21:24 UTC (rev 
15667)
+++ branches/SAMBA_3_0/source/locking/locking.c 2006-05-17 23:15:53 UTC (rev 
15668)
@@ -927,7 +927,7 @@
        e->op_type = op_type;
        e->time.tv_sec = fsp->open_time.tv_sec;
        e->time.tv_usec = fsp->open_time.tv_usec;
-       e->share_file_id = fsp->file_id;
+       e->share_file_id = fsp->fh->file_id;
        e->dev = fsp->dev;
        e->inode = fsp->inode;
 }
@@ -986,28 +986,19 @@
 
 /*******************************************************************
  Check if two share mode entries are identical, ignoring oplock 
- and mid info and desired_access.
+ and mid info and desired_access. (Removed paranoia test - it's
+ not automatically a logic error if they are identical. JRA.)
 ********************************************************************/
 
 static BOOL share_modes_identical(struct share_mode_entry *e1,
                                  struct share_mode_entry *e2)
 {
-#if 1 /* JRA PARANOIA TEST - REMOVE LATER */
-       if (procid_equal(&e1->pid, &e2->pid) &&
-           e1->share_file_id == e2->share_file_id &&
-           e1->dev == e2->dev &&
-           e1->inode == e2->inode &&
-           (e1->share_access) != (e2->share_access)) {
-               DEBUG(0,("PANIC: share_modes_identical: share_mode "
-                        "mismatch (e1 = 0x%x, e2 = 0x%x). Logic error.\n",
-                        (unsigned int)e1->share_access,
-                        (unsigned int)e2->share_access ));
-               smb_panic("PANIC: share_modes_identical logic error.\n");
-       }
-#endif
+       /* We used to check for e1->share_access == e2->share_access here
+          as well as the other fields but 2 different DOS or FCB opens
+          sharing the same share mode entry may validly differ in
+          fsp->share_access field. */
 
        return (procid_equal(&e1->pid, &e2->pid) &&
-               (e1->share_access) == (e2->share_access) &&
                e1->dev == e2->dev &&
                e1->inode == e2->inode &&
                e1->share_file_id == e2->share_file_id );

Modified: branches/SAMBA_3_0/source/smbd/close.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/close.c      2006-05-17 22:21:24 UTC (rev 
15667)
+++ branches/SAMBA_3_0/source/smbd/close.c      2006-05-17 23:15:53 UTC (rev 
15668)
@@ -143,55 +143,16 @@
 }
 
 /****************************************************************************
- Close a file.
-
- close_type can be NORMAL_CLOSE=0,SHUTDOWN_CLOSE,ERROR_CLOSE.
- printing and magic scripts are only run on normal close.
- delete on close is done on normal and shutdown close.
+ Deal with removing a share mode on last close.
 ****************************************************************************/
 
-static int close_normal_file(files_struct *fsp, enum file_close_type 
close_type)
+static int close_remove_share_mode(files_struct *fsp, enum file_close_type 
close_type)
 {
+       connection_struct *conn = fsp->conn;
        BOOL delete_file = False;
-       connection_struct *conn = fsp->conn;
-       int saved_errno = 0;
-       int err = 0;
-       int err1 = 0;
        struct share_mode_lock *lck;
 
-       remove_pending_lock_requests_by_fid(fsp);
-
-       if (fsp->aio_write_behind) {
-               /*
-                * If we're finishing write behind on a close we can get a write
-                * error here, we must remember this.
-                */
-               int ret = wait_for_aio_completion(fsp);
-               if (ret) {
-                       saved_errno = ret;
-                       err1 = -1;
-               }
-       } else {
-               cancel_aio_by_fsp(fsp);
-       }
- 
        /*
-        * If we're flushing on a close we can get a write
-        * error here, we must remember this.
-        */
-
-       if (close_filestruct(fsp) == -1) {
-               saved_errno = errno;
-               err1 = -1;
-       }
-
-       if (fsp->print_file) {
-               print_fsp_end(fsp, close_type);
-               file_free(fsp);
-               return 0;
-       }
-
-       /*
         * Lock the share entries, and determine if we should delete
         * on close. If so delete whilst the lock is still in effect.
         * This prevents race conditions with the file being created. JRA.
@@ -200,12 +161,12 @@
        lck = get_share_mode_lock(NULL, fsp->dev, fsp->inode, NULL, NULL);
 
        if (lck == NULL) {
-               DEBUG(0, ("close_file: Could not get share mode lock for file 
%s\n", fsp->fsp_name));
+               DEBUG(0, ("close_remove_share_mode: Could not get share mode 
lock for file %s\n", fsp->fsp_name));
                return EINVAL;
        }
 
        if (!del_share_mode(lck, fsp)) {
-               DEBUG(0, ("close_file: Could not delete share entry for file 
%s\n", fsp->fsp_name));
+               DEBUG(0, ("close_remove_share_mode: Could not delete share 
entry for file %s\n", fsp->fsp_name));
        }
 
        delete_file = (lck->delete_on_close | lck->initial_delete_on_close);
@@ -236,13 +197,13 @@
                                lck->delete_token) {
                SMB_STRUCT_STAT sbuf;
 
-               DEBUG(5,("close_file: file %s. Delete on close was set - 
deleting file.\n",
+               DEBUG(5,("close_remove_share_mode: file %s. Delete on close was 
set - deleting file.\n",
                        fsp->fsp_name));
 
                /* Become the user who requested the delete. */
 
                if (!push_sec_ctx()) {
-                       smb_panic("close_file: file %s. failed to push 
sec_ctx.\n");
+                       smb_panic("close_remove_share_mode: file %s. failed to 
push sec_ctx.\n");
                }
 
                set_sec_ctx(lck->delete_token->uid,
@@ -253,17 +214,17 @@
 
                /* We can only delete the file if the name we have
                   is still valid and hasn't been renamed. */
-
+       
                if(SMB_VFS_STAT(conn,fsp->fsp_name,&sbuf) != 0) {
-                       DEBUG(5,("close_file: file %s. Delete on close was set "
+                       DEBUG(5,("close_remove_share_mode: file %s. Delete on 
close was set "
                                "and stat failed with error %s\n",
                                fsp->fsp_name, strerror(errno) ));
                } else {
                        if(sbuf.st_dev != fsp->dev || sbuf.st_ino != 
fsp->inode) {
-                               DEBUG(5,("close_file: file %s. Delete on close 
was set and "
+                               DEBUG(5,("close_remove_share_mode: file %s. 
Delete on close was set and "
                                        "dev and/or inode does not match\n",
                                        fsp->fsp_name ));
-                               DEBUG(5,("close_file: file %s. stored dev = %x, 
inode = %.0f "
+                               DEBUG(5,("close_remove_share_mode: file %s. 
stored dev = %x, inode = %.0f "
                                        "stat dev = %x, inode = %.0f\n",
                                        fsp->fsp_name,
                                        (unsigned int)fsp->dev, 
(double)fsp->inode,
@@ -277,21 +238,80 @@
                                 * we log this but not at debug level zero.
                                 */
 
-                               DEBUG(5,("close_file: file %s. Delete on close 
was set "
+                               DEBUG(5,("close_remove_share_mode: file %s. 
Delete on close was set "
                                        "and unlink failed with error %s\n",
                                        fsp->fsp_name, strerror(errno) ));
                        }
                }
                /* unbecome user. */
                pop_sec_ctx();
-
+       
                process_pending_change_notify_queue((time_t)0);
        }
 
        TALLOC_FREE(lck);
+       return 0;
+}
 
-       if(fsp->oplock_type)
+/****************************************************************************
+ Close a file.
+
+ close_type can be NORMAL_CLOSE=0,SHUTDOWN_CLOSE,ERROR_CLOSE.
+ printing and magic scripts are only run on normal close.
+ delete on close is done on normal and shutdown close.
+****************************************************************************/
+
+static int close_normal_file(files_struct *fsp, enum file_close_type 
close_type)
+{
+       connection_struct *conn = fsp->conn;
+       int saved_errno = 0;
+       int err = 0;
+       int err1 = 0;
+
+       remove_pending_lock_requests_by_fid(fsp);
+
+       if (fsp->aio_write_behind) {
+               /*
+                * If we're finishing write behind on a close we can get a write
+                * error here, we must remember this.
+                */
+               int ret = wait_for_aio_completion(fsp);
+               if (ret) {
+                       saved_errno = ret;
+                       err1 = -1;
+               }
+       } else {
+               cancel_aio_by_fsp(fsp);
+       }
+ 
+       /*
+        * If we're flushing on a close we can get a write
+        * error here, we must remember this.
+        */
+
+       if (close_filestruct(fsp) == -1) {
+               saved_errno = errno;
+               err1 = -1;
+       }
+
+       if (fsp->print_file) {
+               print_fsp_end(fsp, close_type);
+               file_free(fsp);
+               return 0;
+       }
+
+       /* If this is an old DOS or FCB open and we have multiple opens on
+          the same handle we only have one share mode. Ensure we only remove
+          the share mode on the last close. */
+
+       if (fsp->fh->ref_count == 1) {
+               /* Should we return on error here... ? */
+               close_remove_share_mode(fsp, close_type);
+       }
+
+       if(fsp->oplock_type) {
                release_file_oplock(fsp);
+       }
 
        locking_close_file(fsp);
 
@@ -323,9 +343,6 @@
                conn->num_files_open,
                (err == -1 || err1 == -1) ? strerror(saved_errno) : ""));
 
-       if (fsp->fsp_name)
-               string_free(&fsp->fsp_name);
-
        file_free(fsp);
 
        if (err == -1 || err1 == -1) {
@@ -410,11 +427,6 @@
         * Do the code common to files and directories.
         */
        close_filestruct(fsp);
-       
-       if (fsp->fsp_name) {
-               string_free(&fsp->fsp_name);
-       }
-       
        file_free(fsp);
        return 0;
 }
@@ -429,10 +441,6 @@
         * Do the code common to files and directories.
         */
        close_filestruct(fsp);
-       
-       if (fsp->fsp_name)
-               string_free(&fsp->fsp_name);
-       
        file_free(fsp);
        return 0;
 }

Modified: branches/SAMBA_3_0/source/smbd/files.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/files.c      2006-05-17 22:21:24 UTC (rev 
15667)
+++ branches/SAMBA_3_0/source/smbd/files.c      2006-05-17 23:15:53 UTC (rev 
15668)
@@ -109,7 +109,7 @@
        fsp->fh->fd = -1;
 
        fsp->conn = conn;
-       fsp->file_id = get_gen_count();
+       fsp->fh->file_id = get_gen_count();
        GetTimeOfDay(&fsp->open_time);
 
        first_file = (i+1) % real_max_open_files;
@@ -238,7 +238,7 @@
 
        for (fsp=Files;fsp;fsp=fsp->next,count++) {
                DEBUG(10,("Files[%d], fnum = %d, name %s, fd = %d, fileid = 
%lu, dev = %x, inode = %.0f\n",
-                       count, fsp->fnum, fsp->fsp_name, fsp->fh->fd, (unsigned 
long)fsp->file_id,
+                       count, fsp->fnum, fsp->fsp_name, fsp->fh->fd, (unsigned 
long)fsp->fh->file_id,
                        (unsigned int)fsp->dev, (double)fsp->inode ));
        }
 }
@@ -277,7 +277,7 @@
                /* We can have a fsp->fh->fd == -1 here as it could be a stat 
open. */
                if (fsp->dev == dev && 
                    fsp->inode == inode &&
-                   fsp->file_id == file_id ) {
+                   fsp->fh->file_id == file_id ) {
                        if (count > 10) {
                                DLIST_PROMOTE(Files, fsp);
                        }
@@ -287,7 +287,7 @@
                            (fsp->oplock_type != FAKE_LEVEL_II_OPLOCK)) {
                                DEBUG(0,("file_find_dif: file %s dev = %x, 
inode = %.0f, file_id = %u \
 oplock_type = %u is a stat open with oplock type !\n", fsp->fsp_name, 
(unsigned int)fsp->dev,
-                                               (double)fsp->inode, (unsigned 
int)fsp->file_id,
+                                               (double)fsp->inode, (unsigned 
int)fsp->fh->file_id,
                                                (unsigned int)fsp->oplock_type 
));
                                smb_panic("file_find_dif\n");
                        }

Modified: branches/SAMBA_3_0/source/smbd/oplock.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/oplock.c     2006-05-17 22:21:24 UTC (rev 
15667)
+++ branches/SAMBA_3_0/source/smbd/oplock.c     2006-05-17 23:15:53 UTC (rev 
15668)
@@ -89,7 +89,7 @@
                /* Put the kernel break info into the message. */
                SDEV_T_VAL(msg,0,fsp->dev);
                SINO_T_VAL(msg,8,fsp->inode);
-               SIVAL(msg,16,fsp->file_id);
+               SIVAL(msg,16,fsp->fh->file_id);
 
                /* Don't need to be root here as we're only ever
                   sending to ourselves. */
@@ -122,7 +122,7 @@
        DEBUG(5,("set_file_oplock: granted oplock on file %s, 0x%x/%.0f/%lu, "
                    "tv_sec = %x, tv_usec = %x\n",
                 fsp->fsp_name, (unsigned int)fsp->dev, (double)fsp->inode,
-                fsp->file_id, (int)fsp->open_time.tv_sec,
+                fsp->fh->file_id, (int)fsp->open_time.tv_sec,
                 (int)fsp->open_time.tv_usec ));
 
        return True;
@@ -333,7 +333,7 @@
                if( DEBUGLVL( 3 ) ) {
                        dbgtext( "initial_break_processing: file %s ", 
fsp->fsp_name );
                        dbgtext( "(dev = %x, inode = %.0f, file_id = %lu) has 
no oplock.\n",
-                               (unsigned int)dev, (double)inode, fsp->file_id 
);
+                               (unsigned int)dev, (double)inode, 
fsp->fh->file_id );
                        dbgtext( "Allowing break to succeed regardless.\n" );
                }
                return NULL;

Modified: branches/SAMBA_3_0/source/smbd/oplock_irix.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/oplock_irix.c        2006-05-17 22:21:24 UTC 
(rev 15667)
+++ branches/SAMBA_3_0/source/smbd/oplock_irix.c        2006-05-17 23:15:53 UTC 
(rev 15668)
@@ -145,7 +145,7 @@
      
        DEBUG(5,("irix_oplock_receive_message: kernel oplock break request "
                 "received for dev = %x, inode = %.0f\n, file_id = %ul",
-                (unsigned int)fsp->dev, (double)fsp->inode, fsp->file_id ));
+                (unsigned int)fsp->dev, (double)fsp->inode, fsp->fh->file_id 
));
 
        return fsp;
 }
@@ -160,18 +160,18 @@
                if(errno != EAGAIN) {
                        DEBUG(0,("irix_set_kernel_oplock: Unable to get kernel 
oplock on file %s, dev = %x, \
 inode = %.0f, file_id = %ul. Error was %s\n", 
-                                fsp->fsp_name, (unsigned int)fsp->dev, 
(double)fsp->inode, fsp->file_id,
+                                fsp->fsp_name, (unsigned int)fsp->dev, 
(double)fsp->inode, fsp->fh->file_id,
                                 strerror(errno) ));
                } else {
                        DEBUG(5,("irix_set_kernel_oplock: Refused oplock on 
file %s, fd = %d, dev = %x, \
 inode = %.0f, file_id = %ul. Another process had the file open.\n",
-                                fsp->fsp_name, fsp->fh->fd, (unsigned 
int)fsp->dev, (double)fsp->inode, fsp->file_id ));
+                                fsp->fsp_name, fsp->fh->fd, (unsigned 
int)fsp->dev, (double)fsp->inode, fsp->fh->file_id ));
                }
                return False;
        }
        
        DEBUG(10,("irix_set_kernel_oplock: got kernel oplock on file %s, dev = 
%x, inode = %.0f, file_id = %ul\n",
-                 fsp->fsp_name, (unsigned int)fsp->dev, (double)fsp->inode, 
fsp->file_id));
+                 fsp->fsp_name, (unsigned int)fsp->dev, (double)fsp->inode, 
fsp->fh->file_id));
 
        return True;
 }
@@ -190,7 +190,7 @@
                int state = sys_fcntl_long(fsp->fh->fd, F_OPLKACK, -1);
                dbgtext("irix_release_kernel_oplock: file %s, dev = %x, inode = 
%.0f file_id = %ul, has kernel \
 oplock state of %x.\n", fsp->fsp_name, (unsigned int)fsp->dev,
-                        (double)fsp->inode, fsp->file_id, state );
+                        (double)fsp->inode, fsp->fh->file_id, state );
        }
 
        /*
@@ -201,7 +201,7 @@
                        dbgtext("irix_release_kernel_oplock: Error when 
removing kernel oplock on file " );
                        dbgtext("%s, dev = %x, inode = %.0f, file_id = %ul. 
Error was %s\n",
                                fsp->fsp_name, (unsigned int)fsp->dev, 
-                               (double)fsp->inode, fsp->file_id, 
strerror(errno) );
+                               (double)fsp->inode, fsp->fh->file_id, 
strerror(errno) );
                }
        }
 }

Modified: branches/SAMBA_3_0/source/smbd/oplock_linux.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/oplock_linux.c       2006-05-17 22:21:24 UTC 
(rev 15667)
+++ branches/SAMBA_3_0/source/smbd/oplock_linux.c       2006-05-17 23:15:53 UTC 
(rev 15668)
@@ -164,7 +164,7 @@
        }
        
        DEBUG(3,("linux_set_kernel_oplock: got kernel oplock on file %s, dev = 
%x, inode = %.0f, file_id = %lu\n",
-                 fsp->fsp_name, (unsigned int)fsp->dev, (double)fsp->inode, 
fsp->file_id));
+                 fsp->fsp_name, (unsigned int)fsp->dev, (double)fsp->inode, 
fsp->fh->file_id));
 
        return True;
 }
@@ -183,7 +183,7 @@
                int state = fcntl(fsp->fh->fd, F_GETLEASE, 0);
                dbgtext("linux_release_kernel_oplock: file %s, dev = %x, inode 
= %.0f file_id = %lu has kernel \
 oplock state of %x.\n", fsp->fsp_name, (unsigned int)fsp->dev,
-                        (double)fsp->inode, fsp->file_id, state );
+                        (double)fsp->inode, fsp->fh->file_id, state );
        }
 
        /*
@@ -194,7 +194,7 @@
                        dbgtext("linux_release_kernel_oplock: Error when 
removing kernel oplock on file " );
                        dbgtext("%s, dev = %x, inode = %.0f, file_id = %lu. 
Error was %s\n",
                                fsp->fsp_name, (unsigned int)fsp->dev, 
-                               (double)fsp->inode, fsp->file_id, 
strerror(errno) );
+                               (double)fsp->inode, fsp->fh->file_id, 
strerror(errno) );
                }
        }
 }

Reply via email to