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) ); } } }
