From 772d1b872d6c075935e46d730ea6781318baf1cb Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 Reviewed-by: Volker Lendecke (cherry picked from commit de7180151fc99893c4763882fecd9d2a623cd061) --- 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 1aae0d3d8f3dc9bd0632b8784952ff3def8cdf01 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 Reviewed-by: Volker Lendecke (cherry picked from commit cb394abe5206dd8ad8a68f157427991b259129a7) --- 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 6d8891551236b95acdbec52df6a2352438b3b120 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 Reviewed-by: Volker Lendecke (cherry picked from commit df83b17c60a08a27a7ddd1d88dc125e15b3ee06d) --- 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