r15668: 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
(This used to be commit fd6fa1d4eaf61783df74ee2da50d331477f06998)

7 files changed, 97 insertions, 98 deletions

diff --git a/source3/include/smb.h b/source3/include/smb.h
index fdeaaad651..f06c551cc0 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -404,6 +404,7 @@ struct fd_handle {
 				 * DELETE_ON_CLOSE is not stored in the share
 				 * mode database.
 				 */
+	unsigned long file_id;
 };
 
 struct timed_event;
@@ -438,7 +439,6 @@ typedef struct files_struct {
 	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;
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 2b6023c0c4..2766b257be 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -927,7 +927,7 @@ static void fill_share_mode_entry(struct share_mode_entry *e,
 	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 @@ void add_deferred_open(struct share_mode_lock *lck, uint16 mid,
 
 /*******************************************************************
  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 );
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index f2d54445b9..8a63d3b227 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -143,54 +143,15 @@ static void notify_deferred_opens(struct share_mode_lock *lck)
 }
 
 /****************************************************************************
- 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)
 {
-	BOOL delete_file = False;
 	connection_struct *conn = fsp->conn;
-	int saved_errno = 0;
-	int err = 0;
-	int err1 = 0;
+	BOOL delete_file = False;
 	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.
@@ -200,12 +161,12 @@ static int close_normal_file(files_struct *fsp, enum file_close_type close_type)
 	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 @@ static int close_normal_file(files_struct *fsp, enum file_close_type close_type)
 				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 @@ static int close_normal_file(files_struct *fsp, enum file_close_type close_type)
 
 		/* 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 @@ static int close_normal_file(files_struct *fsp, enum file_close_type close_type)
 				 * 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;
+}
+
+/****************************************************************************
+ 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;
 
-	if(fsp->oplock_type)
+	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 @@ static int close_normal_file(files_struct *fsp, enum file_close_type close_type)
 		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 @@ static int close_directory(files_struct *fsp, enum file_close_type close_type)
 	 * 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 @@ static int close_stat(files_struct *fsp)
 	 * 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;
 }
diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index ba4baa93b1..53207e876e 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -109,7 +109,7 @@ files_struct *file_new(connection_struct *conn)
 	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 @@ void file_dump_open_table(void)
 
 	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 @@ files_struct *file_find_dif(SMB_DEV_T dev, SMB_INO_T inode, unsigned long file_i
 		/* 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 @@ files_struct *file_find_dif(SMB_DEV_T dev, SMB_INO_T inode, unsigned long file_i
 			    (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");
 			}
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 8254e77e1b..42c64a2843 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -89,7 +89,7 @@ void process_kernel_oplocks(fd_set *pfds)
 		/* 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 @@ BOOL set_file_oplock(files_struct *fsp, int oplock_type)
 	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 @@ static files_struct *initial_break_processing(SMB_DEV_T dev, SMB_INO_T inode, un
 		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;
diff --git a/source3/smbd/oplock_irix.c b/source3/smbd/oplock_irix.c
index 73d582157d..248d902028 100644
--- a/source3/smbd/oplock_irix.c
+++ b/source3/smbd/oplock_irix.c
@@ -145,7 +145,7 @@ static files_struct *irix_oplock_receive_message(fd_set *fds)
      
 	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 @@ static BOOL irix_set_kernel_oplock(files_struct *fsp, int oplock_type)
 		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 @@ static void irix_release_kernel_oplock(files_struct *fsp)
 		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 @@ oplock state of %x.\n", fsp->fsp_name, (unsigned int)fsp->dev,
 			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) );
 		}
 	}
 }
diff --git a/source3/smbd/oplock_linux.c b/source3/smbd/oplock_linux.c
index 9a5eb52e61..f186c13ebd 100644
--- a/source3/smbd/oplock_linux.c
+++ b/source3/smbd/oplock_linux.c
@@ -164,7 +164,7 @@ inode = %.0f. (%s)\n",
 	}
 	
 	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 @@ static void linux_release_kernel_oplock(files_struct *fsp)
 		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 @@ oplock state of %x.\n", fsp->fsp_name, (unsigned int)fsp->dev,
 			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) );
 		}
 	}
 }