The Samba-Bugzilla – Attachment 10070 Details for
Bug 10684
SMB1 blocking locks can fail notification on unlock, causing client timeout.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for master.
bug-10684-master.patch (text/plain), 13.65 KB, created by
Jeremy Allison
on 2014-07-02 19:16:11 UTC
(
hide
)
Description:
git-am fix for master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2014-07-02 19:16:11 UTC
Size:
13.65 KB
patch
obsolete
>From c77c212afd169769084bda9eec7220b8b404347e Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 1 Jul 2014 14:05:08 -0700 >Subject: [PATCH 1/4] s3: smbd: Locking - Remove any pending lock record > *before* attempting to acquire it again. > >The removes duplicate code from the return cases, but is incomplete until >the following patches which re-add a pending lock record if we fail to acquire. >Breaking up the patch this way is clearer (to me at least). > >Bug #10684 - SMB1 blocking locks can fail notification on unlock, causing client timeout. > >https://bugzilla.samba.org/show_bug.cgi?id=10684 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/blocking.c | 59 ++++++++++++++++--------------------------------- > 1 file changed, 19 insertions(+), 40 deletions(-) > >diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c >index 5d198fc..5b1de65 100644 >--- a/source3/smbd/blocking.c >+++ b/source3/smbd/blocking.c >@@ -748,6 +748,7 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) > */ > > for (blr = sconn->smb1.locks.blocking_lock_queue; blr; blr = next) { >+ struct byte_range_lock *br_lck = NULL; > > next = blr->next; > >@@ -767,24 +768,27 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) > SVAL(blr->req->inbuf,smb_flg), > false); > >- if(blocking_lock_record_process(blr)) { >- struct byte_range_lock *br_lck = brl_get_locks( >- talloc_tos(), blr->fsp); >+ /* >+ * Remove the pending lock we're waiting on. >+ * If we need to keep waiting blocking_lock_record_process() >+ * will re-add it. >+ */ > >+ br_lck = brl_get_locks(talloc_tos(), blr->fsp); >+ if (br_lck) { >+ brl_lock_cancel(br_lck, >+ blr->smblctx, >+ messaging_server_id(sconn->msg_ctx), >+ blr->offset, >+ blr->count, >+ blr->lock_flav, >+ blr); >+ TALLOC_FREE(br_lck); >+ } >+ >+ if(blocking_lock_record_process(blr)) { > DEBUG(10, ("BLR_process returned true: cancelling and " > "removing lock. BLR = %p\n", blr)); >- >- if (br_lck) { >- brl_lock_cancel(br_lck, >- blr->smblctx, >- messaging_server_id(sconn->msg_ctx), >- blr->offset, >- blr->count, >- blr->lock_flav, >- blr); >- TALLOC_FREE(br_lck); >- } >- > DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr); > TALLOC_FREE(blr); > continue; >@@ -796,32 +800,7 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) > */ > > if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { >- struct byte_range_lock *br_lck = brl_get_locks( >- talloc_tos(), blr->fsp); >- > DEBUG(10, ("Lock timed out! BLR = %p\n", blr)); >- >- /* >- * Lock expired - throw away all previously >- * obtained locks and return lock error. >- */ >- >- if (br_lck) { >- DEBUG(5,("process_blocking_lock_queue: " >- "pending lock for %s, file %s " >- "timed out.\n", fsp_fnum_dbg(blr->fsp), >- fsp_str_dbg(blr->fsp))); >- >- brl_lock_cancel(br_lck, >- blr->smblctx, >- messaging_server_id(sconn->msg_ctx), >- blr->offset, >- blr->count, >- blr->lock_flav, >- blr); >- TALLOC_FREE(br_lck); >- } >- > blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); > DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr); > TALLOC_FREE(blr); >-- >2.0.0.526.g5318336 > > >From 6a1ee0d54e8d1ad5ca04a36737204059af8fb111 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 2 Jul 2014 11:01:18 -0700 >Subject: [PATCH 2/4] s3: smbd: Locking - add utility function > lock_timed_out(). > >Will be used more extensively in the next patch. > >Bug #10684 - SMB1 blocking locks can fail notification on unlock, causing client timeout. > >https://bugzilla.samba.org/show_bug.cgi?id=10684 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/blocking.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > >diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c >index 5b1de65..bca13b2 100644 >--- a/source3/smbd/blocking.c >+++ b/source3/smbd/blocking.c >@@ -430,6 +430,25 @@ static void blocking_lock_reply_error(struct blocking_lock_record *blr, NTSTATUS > } > > /**************************************************************************** >+ Utility function that returns true if a lock timed out. >+*****************************************************************************/ >+ >+static bool lock_timed_out(const struct blocking_lock_record *blr) >+{ >+ struct timeval tv_curr; >+ >+ if (timeval_is_zero(&blr->expire_time)) { >+ return false; /* Never times out. */ >+ } >+ >+ tv_curr = timeval_current(); >+ if (timeval_compare(&blr->expire_time, &tv_curr) <= 0) { >+ return true; >+ } >+ return false; >+} >+ >+/**************************************************************************** > Attempt to finish off getting all pending blocking locks for a lockingX call. > Returns True if we want to be removed from the list. > *****************************************************************************/ >@@ -735,11 +754,10 @@ static void received_unlock_msg(struct messaging_context *msg, > > void process_blocking_lock_queue(struct smbd_server_connection *sconn) > { >- struct timeval tv_curr = timeval_current(); > struct blocking_lock_record *blr, *next = NULL; > > if (sconn->using_smb2) { >- process_blocking_lock_queue_smb2(sconn, tv_curr); >+ process_blocking_lock_queue_smb2(sconn, timeval_current()); > return; > } > >@@ -799,7 +817,7 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) > * If the time has expired, return a lock error. > */ > >- if (!timeval_is_zero(&blr->expire_time) && timeval_compare(&blr->expire_time, &tv_curr) <= 0) { >+ if (lock_timed_out(blr)) { > DEBUG(10, ("Lock timed out! BLR = %p\n", blr)); > blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); > DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr); >-- >2.0.0.526.g5318336 > > >From 7cb52a29107818bb649ed4f775af8c855ca75501 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 2 Jul 2014 11:14:27 -0700 >Subject: [PATCH 3/4] s3: smbd: Locking - re-add pending lock records if we > fail to acquire a lock (and the lock hasn't timed out). > >In conjunction with the previous patches, this keeps the blocking lock >record and the pending lock records consistent if we are dealing with >multiple blocking lock requests in one SMB1 LockingX request. > >Bug #10684 - SMB1 blocking locks can fail notification on unlock, causing client timeout. > >https://bugzilla.samba.org/show_bug.cgi?id=10684 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/blocking.c | 64 ++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 56 insertions(+), 8 deletions(-) > >diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c >index bca13b2..4f7af3d 100644 >--- a/source3/smbd/blocking.c >+++ b/source3/smbd/blocking.c >@@ -459,8 +459,6 @@ static bool process_lockingX(struct blocking_lock_record *blr) > files_struct *fsp = blr->fsp; > uint16 num_ulocks = SVAL(blr->req->vwv+6, 0); > uint16 num_locks = SVAL(blr->req->vwv+7, 0); >- uint64_t count = (uint64_t)0, offset = (uint64_t)0; >- uint64_t smblctx; > bool large_file_format = (locktype & LOCKING_ANDX_LARGE_FILES); > uint8_t *data; > NTSTATUS status = NT_STATUS_OK; >@@ -477,9 +475,14 @@ static bool process_lockingX(struct blocking_lock_record *blr) > struct byte_range_lock *br_lck = NULL; > bool err; > >- smblctx = get_lock_pid( data, blr->lock_num, large_file_format); >- count = get_lock_count( data, blr->lock_num, large_file_format); >- offset = get_lock_offset( data, blr->lock_num, large_file_format, &err); >+ /* >+ * Ensure the blr record gets updated with >+ * any lock we might end up blocked on. >+ */ >+ >+ blr->smblctx = get_lock_pid( data, blr->lock_num, large_file_format); >+ blr->count = get_lock_count( data, blr->lock_num, large_file_format); >+ blr->offset = get_lock_offset( data, blr->lock_num, large_file_format, &err); > > /* > * We know err cannot be set as if it was the lock >@@ -488,9 +491,9 @@ static bool process_lockingX(struct blocking_lock_record *blr) > errno = 0; > br_lck = do_lock(fsp->conn->sconn->msg_ctx, > fsp, >- smblctx, >- count, >- offset, >+ blr->smblctx, >+ blr->count, >+ blr->offset, > ((locktype & LOCKING_ANDX_SHARED_LOCK) ? > READ_LOCK : WRITE_LOCK), > WINDOWS_LOCK, >@@ -499,6 +502,28 @@ static bool process_lockingX(struct blocking_lock_record *blr) > &blr->blocking_smblctx, > blr); > >+ if (ERROR_WAS_LOCK_DENIED(status) && !lock_timed_out(blr)) { >+ /* >+ * Re-add the pending lock entry if it's still active. >+ */ >+ NTSTATUS status1 = brl_lock(fsp->conn->sconn->msg_ctx, >+ br_lck, >+ blr->smblctx, >+ messaging_server_id(fsp->conn->sconn->msg_ctx), >+ blr->offset, >+ blr->count, >+ locktype == READ_LOCK ? >+ PENDING_READ_LOCK : PENDING_WRITE_LOCK, >+ blr->lock_flav, >+ true, /* Blocking lock. */ >+ NULL, >+ blr); >+ >+ if (!NT_STATUS_IS_OK(status1)) { >+ DEBUG(0,("failed to add PENDING_LOCK record.\n")); >+ } >+ } >+ > TALLOC_FREE(br_lck); > > if (NT_STATUS_IS_ERR(status)) { >@@ -563,6 +588,29 @@ static bool process_trans2(struct blocking_lock_record *blr) > &status, > &blr->blocking_smblctx, > blr); >+ >+ if (ERROR_WAS_LOCK_DENIED(status) && !lock_timed_out(blr)) { >+ /* >+ * Re-add the pending lock entry. >+ */ >+ NTSTATUS status1 = brl_lock(blr->fsp->conn->sconn->msg_ctx, >+ br_lck, >+ blr->smblctx, >+ messaging_server_id(blr->fsp->conn->sconn->msg_ctx), >+ blr->offset, >+ blr->count, >+ blr->lock_type == READ_LOCK ? >+ PENDING_READ_LOCK : PENDING_WRITE_LOCK, >+ blr->lock_flav, >+ true, /* Blocking lock. */ >+ NULL, >+ blr); >+ >+ if (!NT_STATUS_IS_OK(status1)) { >+ DEBUG(0,("failed to add PENDING_LOCK record.\n")); >+ } >+ } >+ > TALLOC_FREE(br_lck); > > if (!NT_STATUS_IS_OK(status)) { >-- >2.0.0.526.g5318336 > > >From 937531d363a96847d3c21af2fe19f0a33b1abde3 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 1 Jul 2014 12:05:07 -0700 >Subject: [PATCH 4/4] s4: smbtorture: Add multi-lock test. Regression test for > bug #10684. > >Bug #10684 - SMB1 blocking locks can fail notification on unlock, causing client timeout. > >https://bugzilla.samba.org/show_bug.cgi?id=10684 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source4/torture/raw/lock.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 97 insertions(+) > >diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c >index a31a4d0..8dc2434 100644 >--- a/source4/torture/raw/lock.c >+++ b/source4/torture/raw/lock.c >@@ -2262,6 +2262,102 @@ done: > smbcli_deltree(cli->tree, BASEDIR); > return ret; > } >+/* >+ test multi Locking&X operation >+*/ >+static bool test_multilock(struct torture_context *tctx, >+ struct smbcli_state *cli) >+{ >+ union smb_lock io; >+ struct smb_lock_entry lock[2]; >+ NTSTATUS status; >+ bool ret = true; >+ int fnum; >+ const char *fname = BASEDIR "\\multilock_test.txt"; >+ time_t t; >+ struct smbcli_request *req; >+ struct smbcli_session_options options; >+ >+ torture_assert(tctx, torture_setup_dir(cli, BASEDIR), "Failed to setup up test directory: " BASEDIR); >+ >+ lpcfg_smbcli_session_options(tctx->lp_ctx, &options); >+ >+ torture_comment(tctx, "Testing LOCKING_ANDX multi-lock\n"); >+ io.generic.level = RAW_LOCK_LOCKX; >+ >+ /* Create the test file. */ >+ fnum = smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE); >+ torture_assert(tctx,(fnum != -1), talloc_asprintf(tctx, >+ "Failed to create %s - %s\n", >+ fname, smbcli_errstr(cli->tree))); >+ >+ /* >+ * Lock regions 100->109, 110->119 as >+ * two separate write locks in one request. >+ */ >+ io.lockx.level = RAW_LOCK_LOCKX; >+ io.lockx.in.file.fnum = fnum; >+ io.lockx.in.mode = LOCKING_ANDX_LARGE_FILES; >+ io.lockx.in.timeout = 0; >+ io.lockx.in.ulock_cnt = 0; >+ io.lockx.in.lock_cnt = 2; >+ io.lockx.in.mode = LOCKING_ANDX_EXCLUSIVE_LOCK; >+ lock[0].pid = cli->session->pid; >+ lock[0].offset = 100; >+ lock[0].count = 10; >+ lock[1].pid = cli->session->pid; >+ lock[1].offset = 120; >+ lock[1].count = 10; >+ io.lockx.in.locks = &lock[0]; >+ status = smb_raw_lock(cli->tree, &io); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* >+ * Now request the same locks on a different >+ * context as blocking locks with infinite timeout. >+ */ >+ >+ io.lockx.in.timeout = -1; >+ lock[0].pid = cli->session->pid+1; >+ lock[1].pid = cli->session->pid+1; >+ req = smb_raw_lock_send(cli->tree, &io); >+ torture_assert(tctx,(req != NULL), talloc_asprintf(tctx, >+ "Failed to setup timed locks (%s)\n", __location__)); >+ >+ /* Unlock lock[0] */ >+ io.lockx.in.timeout = 0; >+ io.lockx.in.ulock_cnt = 1; >+ io.lockx.in.lock_cnt = 0; >+ io.lockx.in.locks = &lock[0]; >+ lock[0].pid = cli->session->pid; >+ status = smb_raw_lock(cli->tree, &io); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* Start the clock. */ >+ t = time_mono(NULL); >+ >+ /* Unlock lock[1] */ >+ io.lockx.in.timeout = 0; >+ io.lockx.in.ulock_cnt = 1; >+ io.lockx.in.lock_cnt = 0; >+ io.lockx.in.locks = &lock[1]; >+ lock[1].pid = cli->session->pid; >+ status = smb_raw_lock(cli->tree, &io); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* receive the successful blocked lock requests */ >+ status = smbcli_request_simple_recv(req); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* Fail if this took more than 2 seconds. */ >+ torture_assert(tctx,!(time_mono(NULL) > t+2), talloc_asprintf(tctx, >+ "Blocking locks were not granted immediately (%s)\n", >+ __location__)); >+done: >+ smb_raw_exit(cli->session); >+ smbcli_deltree(cli->tree, BASEDIR); >+ return ret; >+} > > /* > basic testing of lock calls >@@ -2283,6 +2379,7 @@ struct torture_suite *torture_raw_lock(TALLOC_CTX *mem_ctx) > test_multiple_unlock); > torture_suite_add_1smb_test(suite, "zerobytelocks", test_zerobytelocks); > torture_suite_add_1smb_test(suite, "zerobyteread", test_zerobyteread); >+ torture_suite_add_1smb_test(suite, "multilock", test_multilock); > > return suite; > } >-- >2.0.0.526.g5318336 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=10070">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 10684
:
10064
|
10070
|
10072
|
10076