r17125: Drastic problems require drastic solutions. There's

no way to get all the cases where kernel oplocks are
on and we can't open the file and get the correct
semantics (think about the open with truncate with
an attribute only open - we'd need a vfs change to
add the truncate(fname, len) call). So always drop
the share mode lock before doing any real fd opens and
then re-acquire it afterwards. We're already dealing
with the race in the create case, and we deal with
any other races in the same way. Volker, please
examine *carefully* :-). This should fix the problems
people reported with kernel oplocks being on.
Jeremy.
(This used to be commit 8171c4c404e9f382880c65daa0232f89e560f399)

1 files changed, 73 insertions, 76 deletions

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 7c04cdbe7c..b13b4f2a6c 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1403,6 +1403,8 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		}
 
 		if (!NT_STATUS_IS_OK(status)) {
+			uint32 can_access_mask;
+			BOOL can_access = True;
 
 			SMB_ASSERT(NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION));
 
@@ -1434,29 +1436,20 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			 * MS-Access. If a file open will fail due to share
 			 * permissions and also for security (access) reasons,
 			 * we need to return the access failed error, not the
-			 * share error. This means we must attempt to open the
-			 * file anyway in order to get the UNIX access error -
-			 * even if we're going to fail the open for share
-			 * reasons. This is bad, as we're burning another fd
-			 * if there are existing locks but there's nothing
-			 * else we can do. We also ensure we're not going to
-			 * create or tuncate the file as we only want an
-			 * access decision at this stage. JRA.
+			 * share error. We can't open the file due to kernel
+			 * oplock deadlock (it's possible we failed above on
+			 * the open_mode_check()) so use a userspace check.
 			 */
-			errno = 0;
-			fsp_open = open_file(fsp,conn,fname,psbuf,
-					     flags|(flags2&~(O_TRUNC|O_CREAT)),
-					     unx_mode,access_mask);
-
-			DEBUG(4,("open_file_ntcreate : share_mode deny - "
-				 "calling open_file with flags=0x%X "
-				 "flags2=0x%X mode=0%o returned %s\n",
-				 flags, (flags2&~(O_TRUNC|O_CREAT)),
-				 (unsigned int)unx_mode, nt_errstr(fsp_open)));
-
-			if (!NT_STATUS_IS_OK(fsp_open) && errno) {
-				/* Default error. */
-				status = NT_STATUS_ACCESS_DENIED;
+
+			if (flags & O_RDWR) {
+				can_access_mask = FILE_READ_DATA|FILE_WRITE_DATA;
+			} else {
+				can_access_mask = FILE_READ_DATA;
+			}
+
+			if (((flags & O_RDWR) && !CAN_WRITE(conn)) ||
+					!can_access_file(conn,fname,psbuf,can_access_mask)) {
+				can_access = False;
 			}
 
 			/* 
@@ -1503,13 +1496,14 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 			}
 
 			TALLOC_FREE(lck);
-			if (NT_STATUS_IS_OK(fsp_open)) {
-				fd_close(conn, fsp);
+			if (can_access) {
 				/*
 				 * We have detected a sharing violation here
 				 * so return the correct error code
 				 */
 				status = NT_STATUS_SHARING_VIOLATION;
+			} else {
+				status = NT_STATUS_ACCESS_DENIED;
 			}
 			file_free(fsp);
 			return status;
@@ -1536,6 +1530,14 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		 (unsigned int)flags, (unsigned int)flags2,
 		 (unsigned int)unx_mode));
 
+	/* Drop the lock before doing any real file access. Allows kernel
+	   oplock breaks to be processed. Handle any races after the open
+	   call when we re-acquire the lock. */
+
+	if (lck) {
+		TALLOC_FREE(lck);
+	}
+
 	/*
 	 * open_file strips any O_TRUNC flags itself.
 	 */
@@ -1551,70 +1553,65 @@ NTSTATUS open_file_ntcreate(connection_struct *conn,
 		return fsp_open;
 	}
 
-	if (!file_existed) { 
-
-		/*
-		 * Deal with the race condition where two smbd's detect the
-		 * file doesn't exist and do the create at the same time. One
-		 * of them will win and set a share mode, the other (ie. this
-		 * one) should check if the requested share mode for this
-		 * create is allowed.
-		 */
-
-		/*
-		 * Now the file exists and fsp is successfully opened,
-		 * fsp->dev and fsp->inode are valid and should replace the
-		 * dev=0,inode=0 from a non existent file. Spotted by
-		 * Nadav Danieli <nadavd@exanet.com>. JRA.
-		 */
+	/*
+	 * Deal with the race condition where two smbd's detect the
+	 * file doesn't exist and do the create at the same time. One
+	 * of them will win and set a share mode, the other (ie. this
+	 * one) should check if the requested share mode for this
+	 * create is allowed.
+	 */
 
-		dev = fsp->dev;
-		inode = fsp->inode;
+	/*
+	 * Now the file exists and fsp is successfully opened,
+	 * fsp->dev and fsp->inode are valid and should replace the
+	 * dev=0,inode=0 from a non existent file. Spotted by
+	 * Nadav Danieli <nadavd@exanet.com>. JRA.
+	 */
 
-		lck = get_share_mode_lock(NULL, dev, inode,
-					conn->connectpath,
-					fname);
+	dev = fsp->dev;
+	inode = fsp->inode;
 
-		if (lck == NULL) {
-			DEBUG(0, ("open_file_ntcreate: Could not get share mode lock for %s\n", fname));
-			fd_close(conn, fsp);
-			file_free(fsp);
-			return NT_STATUS_SHARING_VIOLATION;
-		}
+	lck = get_share_mode_lock(NULL, dev, inode,
+				conn->connectpath,
+				fname);
 
-		status = open_mode_check(conn, fname, lck,
-					 access_mask, share_access,
-					 create_options, &file_existed);
+	if (lck == NULL) {
+		DEBUG(0, ("open_file_ntcreate: Could not get share mode lock for %s\n", fname));
+		fd_close(conn, fsp);
+		file_free(fsp);
+		return NT_STATUS_SHARING_VIOLATION;
+	}
 
-		if (!NT_STATUS_IS_OK(status)) {
-			struct deferred_open_record state;
+	status = open_mode_check(conn, fname, lck,
+				 access_mask, share_access,
+				 create_options, &file_existed);
 
-			fd_close(conn, fsp);
-			file_free(fsp);
+	if (!NT_STATUS_IS_OK(status)) {
+		struct deferred_open_record state;
 
-			state.delayed_for_oplocks = False;
-			state.dev = dev;
-			state.inode = inode;
+		fd_close(conn, fsp);
+		file_free(fsp);
 
-			/* Do it all over again immediately. In the second
-			 * round we will find that the file existed and handle
-			 * the DELETE_PENDING and FCB cases correctly. No need
-			 * to duplicate the code here. Essentially this is a
-			 * "goto top of this function", but don't tell
-			 * anybody... */
+		state.delayed_for_oplocks = False;
+		state.dev = dev;
+		state.inode = inode;
 
-			defer_open(lck, request_time, timeval_zero(),
-				   &state);
-			TALLOC_FREE(lck);
-			return status;
-		}
+		/* Do it all over again immediately. In the second
+		 * round we will find that the file existed and handle
+		 * the DELETE_PENDING and FCB cases correctly. No need
+		 * to duplicate the code here. Essentially this is a
+		 * "goto top of this function", but don't tell
+		 * anybody... */
 
-		/*
-		 * We exit this block with the share entry *locked*.....
-		 */
+		defer_open(lck, request_time, timeval_zero(),
+			   &state);
+		TALLOC_FREE(lck);
+		return status;
 	}
 
-	SMB_ASSERT(lck != NULL);
+	/*
+	 * The share entry is again *locked*.....
+	 */
 
 	/* note that we ignore failure for the following. It is
            basically a hack for NFS, and NFS will never set one of