oplock_break_level2 with unlocked share enty?

[Jeremy Allison]

On Wed, Jan 26, 2005 at 12:07:48AM +0200, Nadav Danieli wrote:
> On Tue, 2005-01-25 at 23:33, Jeremy Allison wrote:
> 
> > On Tue, Jan 25, 2005 at 09:12:16PM +0200, Nadav Danieli wrote:
> > > Hi All,
> > > 
> > > I appears that when breaking our own oplock, oplock level II may be
> > > removed while the share entry is unlocked.
> > > Trace is as follow:
> > > 
> > > oplock_break_level2
> > > smbd/oplock.c:647
> > > oplock_break
> > > smbd/oplock.c:722
> > > request_oplock_break
> > > smbd/oplock.c:994
> > > open_mode_check
> > > smbd/open.c:668
> > 
> > Yes, I've confirmed this is a logic bug. I'll fix it for 3.0.11 proper.
> > What version of Samba are you running ? I'll send you a point patch.
> > 
> > Thanks a *lot* for finding this !
> > 
> > Jeremy.
> 
> 
> The trivial place to acquire the lock is request_oplock_break which is
> called from only 2 functions, and the other,
> release_level_2_oplocks_on_change, already bypass directly to
> oplock_break_level2 for local oplock breaks.

Ok, can you try the attached patch ? I think this fixes it correctly.

Thanks,

	Jeremy.
-------------- next part --------------
Index: smbd/oplock.c
===================================================================
--- smbd/oplock.c	(revision 5000)
+++ smbd/oplock.c	(working copy)
@@ -598,13 +598,13 @@
 
 /****************************************************************************
  Process a level II oplock break directly.
+ We must call this function with the share mode entry locked.
 ****************************************************************************/
 
-BOOL oplock_break_level2(files_struct *fsp, BOOL local_request, int token)
+static BOOL oplock_break_level2(files_struct *fsp, BOOL local_request)
 {
 	extern uint32 global_client_caps;
 	char outbuf[128];
-	BOOL got_lock = False;
 	SMB_DEV_T dev = fsp->dev;
 	SMB_INO_T inode = fsp->inode;
 
@@ -644,25 +644,16 @@
 	/*
 	 * Now we must update the shared memory structure to tell
 	 * everyone else we no longer have a level II oplock on 
-	 * this open file. If local_request is true then token is
-	 * the existing lock on the shared memory area.
+	 * this open file. We must call this function with the share mode
+	 * entry locked so we can change the entry directly.
 	 */
 
-	if(!local_request && lock_share_entry_fsp(fsp) == False) {
-		DEBUG(0,("oplock_break_level2: unable to lock share entry for file %s\n", fsp->fsp_name ));
-	} else {
-		got_lock = True;
-	}
-
 	if(remove_share_oplock(fsp)==False) {
 		DEBUG(0,("oplock_break_level2: unable to remove level II oplock for file %s\n", fsp->fsp_name ));
 	}
 
 	release_file_oplock(fsp);
 
-	if (!local_request && got_lock)
-		unlock_share_entry_fsp(fsp);
-
 	if(level_II_oplocks_open < 0) {
 		DEBUG(0,("oplock_break_level2: level_II_oplocks_open < 0 (%d). PANIC ERROR\n",
 			level_II_oplocks_open));
@@ -680,6 +671,7 @@
 
 /****************************************************************************
  Process an oplock break directly.
+ This is always called with the share mode lock *NOT* held.
 ****************************************************************************/
 
 static BOOL oplock_break(SMB_DEV_T dev, SMB_INO_T inode, unsigned long file_id, BOOL local_request)
@@ -708,8 +700,18 @@
 	 * Deal with a level II oplock going break to none separately.
 	 */
 
-	if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type))
-		return oplock_break_level2(fsp, local_request, -1);
+	if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type)) {
+		BOOL ret;
+		/* We must always call oplock_break_level2() with
+		   the share mode entry locked. */
+		if (lock_share_entry_fsp(fsp) == False) {
+			DEBUG(0,("oplock_break: unable to lock share entry for file %s\n", fsp->fsp_name ));
+			return False;
+		}
+		ret = oplock_break_level2(fsp, local_request);
+		unlock_share_entry_fsp(fsp);
+		return ret;
+	}
 
 	/* Mark the oplock break as sent - we don't want to send twice! */
 	if (fsp->sent_oplock_break) {
@@ -944,6 +946,7 @@
 /****************************************************************************
 Send an oplock break message to another smbd process. If the oplock is held 
 by the local smbd then call the oplock break function directly.
+Note this function is always called with the share mode lock *NOT* held.
 ****************************************************************************/
 
 BOOL request_oplock_break(share_mode_entry *share_entry, BOOL async)
@@ -1155,7 +1158,6 @@
 {
 	share_mode_entry *share_list = NULL;
 	pid_t pid = sys_getpid();
-	int token = -1;
 	int num_share_modes = 0;
 	int i;
 
@@ -1222,7 +1224,7 @@
 
 			DEBUG(10,("release_level_2_oplocks_on_change: breaking our own oplock.\n"));
 
-			oplock_break_level2(new_fsp, True, token);
+			oplock_break_level2(new_fsp, True);
 
 		} else {