From 76dd28b48ba0c6618c0df8833c30ab5e81247cd8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 2 Jul 2014 17:25:22 -0700 Subject: s3: smbd: Locking - convert to using utility macro used elsewhere. 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 Reviewed-by: Volker Lendecke (cherry picked from commit 517fa80bd385c6adcfee03ea6b25599013ad88f5) --- source3/smbd/blocking.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'source3/smbd/blocking.c') diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 5f6dda318e6..54ce016aadd 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -500,8 +500,7 @@ static bool process_lockingX(struct blocking_lock_record *blr) return True; } - if (!NT_STATUS_EQUAL(status,NT_STATUS_LOCK_NOT_GRANTED) && - !NT_STATUS_EQUAL(status,NT_STATUS_FILE_LOCK_CONFLICT)) { + if (!ERROR_WAS_LOCK_DENIED(status)) { /* * We have other than a "can't get lock" * error. Free any locks we had and return an error. -- cgit v1.2.1 From 6484211b6815765cf6e1dcb5b884e82c6edc8bae Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 2 Jul 2014 20:18:42 -0700 Subject: s3: smbd: Locking - add and use utility function lock_timed_out(). 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 Reviewed-by: Volker Lendecke (cherry picked from commit 12be57ef3b2d1b670be7a83f29cd580938030015) --- source3/smbd/blocking.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) (limited to 'source3/smbd/blocking.c') diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 54ce016aadd..303585069d4 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -429,6 +429,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. @@ -734,11 +753,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; } @@ -794,7 +812,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)) { struct byte_range_lock *br_lck = brl_get_locks( talloc_tos(), blr->fsp); -- cgit v1.2.1 From 01753e8409497f8f82dd637a0ad8402da9464d5c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 2 Jul 2014 20:40:49 -0700 Subject: s3: smbd: Locking - treat lock timeout the same as any other error. Allows the special case in process_blocking_lock_queue() that talks back to the client to be removed. 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 Reviewed-by: Volker Lendecke (cherry picked from commit cc9de6eb091159a84228b988c49261c46c301233) --- source3/smbd/blocking.c | 91 +++++++++++++++++++++---------------------------- 1 file changed, 38 insertions(+), 53 deletions(-) (limited to 'source3/smbd/blocking.c') diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index 303585069d4..b6edb65c236 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -464,6 +464,7 @@ static bool process_lockingX(struct blocking_lock_record *blr) bool large_file_format = (locktype & LOCKING_ANDX_LARGE_FILES); uint8_t *data; NTSTATUS status = NT_STATUS_OK; + bool lock_timeout = lock_timed_out(blr); data = discard_const_p(uint8_t, blr->req->buf) + ((large_file_format ? 20 : 10)*num_ulocks); @@ -529,6 +530,14 @@ static bool process_lockingX(struct blocking_lock_record *blr) return True; } + /* + * Return an error to the client if we timed out. + */ + if (lock_timeout) { + blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); + return true; + } + /* * Still can't get all the locks - keep waiting. */ @@ -550,6 +559,8 @@ static bool process_trans2(struct blocking_lock_record *blr) { char params[2]; NTSTATUS status; + bool lock_timeout = lock_timed_out(blr); + struct byte_range_lock *br_lck = do_lock( blr->fsp->conn->sconn->msg_ctx, blr->fsp, @@ -566,6 +577,15 @@ static bool process_trans2(struct blocking_lock_record *blr) if (!NT_STATUS_IS_OK(status)) { if (ERROR_WAS_LOCK_DENIED(status)) { + if (lock_timeout) { + /* + * Return an error if we timed out + * and return true to get dequeued. + */ + blocking_lock_reply_error(blr, + NT_STATUS_FILE_LOCK_CONFLICT); + return true; + } /* Still can't get the lock, just keep waiting. */ return False; } @@ -765,6 +785,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; @@ -784,65 +805,29 @@ 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); - - 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); + if(!blocking_lock_record_process(blr)) { + DEBUG(10, ("still waiting for lock. BLR = %p\n", blr)); continue; } - /* - * We couldn't get the locks for this record on the list. - * If the time has expired, return a lock error. - */ - - if (lock_timed_out(blr)) { - struct byte_range_lock *br_lck = brl_get_locks( - talloc_tos(), blr->fsp); - - DEBUG(10, ("Lock timed out! BLR = %p\n", blr)); + br_lck = brl_get_locks(talloc_tos(), blr->fsp); - /* - * 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); - } + DEBUG(10, ("BLR_process returned true: cancelling and " + "removing lock. BLR = %p\n", blr)); - blocking_lock_reply_error(blr,NT_STATUS_FILE_LOCK_CONFLICT); - DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr); - TALLOC_FREE(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); } recalc_brl_timeout(sconn); -- cgit v1.2.1 From 2f118b639cf1eb1579b8554d42b18b31a5b222dd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 2 Jul 2014 20:51:24 -0700 Subject: s3: smbd: Locking - re-add pending lock records if we fail to acquire a lock (and the lock hasn't timed out). Keep the blocking lock record and the pending lock records consistent if we are dealing with multiple blocking lock requests in one SMB1 LockingX request. Ensure we re-add the records under the record lock, to avoid race conditions. 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 Reviewed-by: Volker Lendecke (cherry picked from commit 954401f8b2b16b3e2ef9655e8ce94d657becce36) --- source3/smbd/blocking.c | 97 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 17 deletions(-) (limited to 'source3/smbd/blocking.c') diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index b6edb65c236..d5b8347469a 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; @@ -478,9 +476,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 @@ -489,9 +492,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, @@ -500,6 +503,34 @@ static bool process_lockingX(struct blocking_lock_record *blr) &blr->blocking_smblctx, blr); + if (ERROR_WAS_LOCK_DENIED(status) && !lock_timeout) { + /* + * If we didn't timeout, but still need to wait, + * re-add the pending lock entry whilst holding + * the brlock db lock. + */ + 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_ERR(status)) { @@ -573,6 +604,33 @@ static bool process_trans2(struct blocking_lock_record *blr) &status, &blr->blocking_smblctx, blr); + if (ERROR_WAS_LOCK_DENIED(status) && !lock_timeout) { + /* + * If we didn't timeout, but still need to wait, + * re-add the pending lock entry whilst holding + * the brlock db lock. + */ + 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)) { @@ -805,16 +863,13 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) SVAL(blr->req->inbuf,smb_flg), false); - if(!blocking_lock_record_process(blr)) { - DEBUG(10, ("still waiting for lock. BLR = %p\n", blr)); - continue; - } + /* + * 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); - - DEBUG(10, ("BLR_process returned true: cancelling and " - "removing lock. BLR = %p\n", blr)); - if (br_lck) { brl_lock_cancel(br_lck, blr->smblctx, @@ -823,8 +878,16 @@ void process_blocking_lock_queue(struct smbd_server_connection *sconn) blr->count, blr->lock_flav, blr); - TALLOC_FREE(br_lck); } + TALLOC_FREE(br_lck); + + if(!blocking_lock_record_process(blr)) { + DEBUG(10, ("still waiting for lock. BLR = %p\n", blr)); + continue; + } + + DEBUG(10, ("BLR_process returned true: removing BLR = %p\n", + blr)); DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr); TALLOC_FREE(blr); -- cgit v1.2.1