Attachment 12359 Details for Bug 12139 – Proposed patch for master.

[patch] Proposed patch for master.

bug-12139-master (text/plain), 6.46 KB, created by Jeremy Allison on 2016-08-10 21:46:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2016-08-10 21:46:49 UTC

Size: 6.46 KB

patch

obsolete

>From 286ff39cf7d76961be46d7b9436b0b0ca173fb0e Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 10 Aug 2016 14:35:42 -0700
>Subject: [PATCH 1/3] smbd: oplock: Fixup debug messages inside
> remove_oplock().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12139
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/oplock.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
>diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
>index be20dee..423ebba 100644
>--- a/source3/smbd/oplock.c
>+++ b/source3/smbd/oplock.c
>@@ -233,32 +233,31 @@ bool remove_oplock(files_struct *fsp)
> 	bool ret;
> 	struct share_mode_lock *lck;
> 
>-	DEBUG(10, ("remove_oplock called for %s\n",
>-		   fsp_str_dbg(fsp)));
>+	DBG_DEBUG("remove_oplock called for %s\n", fsp_str_dbg(fsp));
> 
> 	/* Remove the oplock flag from the sharemode. */
> 	lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
> 	if (lck == NULL) {
>-		DEBUG(0,("remove_oplock: failed to lock share entry for "
>-			 "file %s\n", fsp_str_dbg(fsp)));
>-		return False;
>+		DBG_ERR("failed to lock share entry for "
>+			 "file %s\n", fsp_str_dbg(fsp));
>+		return false;
> 	}
> 
> 	ret = remove_share_oplock(lck, fsp);
> 	if (!ret) {
>-		DEBUG(0,("remove_oplock: failed to remove share oplock for "
>+		DBG_ERR("failed to remove share oplock for "
> 			 "file %s, %s, %s\n",
> 			 fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>-			 file_id_string_tos(&fsp->file_id)));
>+			 file_id_string_tos(&fsp->file_id));
> 	}
> 	release_file_oplock(fsp);
> 
> 	ret = update_num_read_oplocks(fsp, lck);
> 	if (!ret) {
>-		DEBUG(0, ("%s: update_num_read_oplocks failed for "
>+		DBG_ERR("update_num_read_oplocks failed for "
> 			 "file %s, %s, %s\n",
>-			  __func__, fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>-			 file_id_string_tos(&fsp->file_id)));
>+			 fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>+			 file_id_string_tos(&fsp->file_id));
> 	}
> 
> 	TALLOC_FREE(lck);
>-- 
>2.8.0.rc3.226.g39d4020
>
>
>From 5d99ad302f32b57c8844bcb5fbc9be76884a8ee5 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 10 Aug 2016 14:39:52 -0700
>Subject: [PATCH 2/3] smbd: oplock: Factor out internals of remove_oplock()
> into new remove_oplock_under_lock().
>
>Allows this to be called elsewhere.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12139
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/oplock.c | 45 +++++++++++++++++++++++++++++----------------
> source3/smbd/proto.h  |  1 +
> 2 files changed, 30 insertions(+), 16 deletions(-)
>
>diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
>index 423ebba..ff87d9e 100644
>--- a/source3/smbd/oplock.c
>+++ b/source3/smbd/oplock.c
>@@ -223,6 +223,34 @@ bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck)
> }
> 
> /****************************************************************************
>+ Remove a file oplock with lock already held. Copes with level II and exclusive.
>+****************************************************************************/
>+
>+bool remove_oplock_under_lock(files_struct *fsp, struct share_mode_lock *lck)
>+{
>+	bool ret;
>+
>+	ret = remove_share_oplock(lck, fsp);
>+	if (!ret) {
>+		DBG_ERR("failed to remove share oplock for "
>+			"file %s, %s, %s\n",
>+			fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>+			file_id_string_tos(&fsp->file_id));
>+	}
>+	release_file_oplock(fsp);
>+
>+	ret = update_num_read_oplocks(fsp, lck);
>+	if (!ret) {
>+		DBG_ERR("update_num_read_oplocks failed for "
>+			"file %s, %s, %s\n",
>+			fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>+			file_id_string_tos(&fsp->file_id));
>+	}
>+
>+	return ret;
>+}
>+
>+/****************************************************************************
>  Remove a file oplock. Copes with level II and exclusive.
>  Locks then unlocks the share mode lock. Client can decide to go directly
>  to none even if a "break-to-level II" was sent.
>@@ -243,22 +271,7 @@ bool remove_oplock(files_struct *fsp)
> 		return false;
> 	}
> 
>-	ret = remove_share_oplock(lck, fsp);
>-	if (!ret) {
>-		DBG_ERR("failed to remove share oplock for "
>-			 "file %s, %s, %s\n",
>-			 fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>-			 file_id_string_tos(&fsp->file_id));
>-	}
>-	release_file_oplock(fsp);
>-
>-	ret = update_num_read_oplocks(fsp, lck);
>-	if (!ret) {
>-		DBG_ERR("update_num_read_oplocks failed for "
>-			 "file %s, %s, %s\n",
>-			 fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
>-			 file_id_string_tos(&fsp->file_id));
>-	}
>+	ret = remove_oplock_under_lock(fsp, lck);
> 
> 	TALLOC_FREE(lck);
> 	return ret;
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index 26fec95..97fe577 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -708,6 +708,7 @@ bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck);
> 
> void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp);
> NTSTATUS set_file_oplock(files_struct *fsp);
>+bool remove_oplock_under_lock(files_struct *fsp, struct share_mode_lock *lck);
> bool remove_oplock(files_struct *fsp);
> bool downgrade_oplock(files_struct *fsp);
> bool fsp_lease_update(struct share_mode_lock *lck,
>-- 
>2.8.0.rc3.226.g39d4020
>
>
>From fa8e52e586af23b1182f18cdfe75c3877f9c89d2 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 10 Aug 2016 14:42:07 -0700
>Subject: [PATCH 3/3] s3: oplock: Fix race condition when closing an oplocked
> file.
>
>We must send the 'oplock released' message whilst the lock
>is held in the close path. Otherwise the messaged smbd can
>race with the share mode delete.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12139
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/close.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/source3/smbd/close.c b/source3/smbd/close.c
>index 3c4b9b1..22bd361 100644
>--- a/source3/smbd/close.c
>+++ b/source3/smbd/close.c
>@@ -273,6 +273,11 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
>+	/* Remove the oplock before potentially deleting the file. */
>+	if(fsp->oplock_type) {
>+		remove_oplock_under_lock(fsp, lck);
>+	}
>+
> 	if (fsp->write_time_forced) {
> 		DEBUG(10,("close_remove_share_mode: write time forced "
> 			"for file %s\n",
>@@ -741,11 +746,6 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp,
> 		return NT_STATUS_OK;
> 	}
> 
>-	/* Remove the oplock before potentially deleting the file. */
>-	if(fsp->oplock_type) {
>-		remove_oplock(fsp);
>-	}
>-
> 	/* 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. */
>-- 
>2.8.0.rc3.226.g39d4020
>

Actions: View

Attachments on bug 12139: 12352 | 12353 | 12354 | 12355 | 12359 | 12360 | 12362