[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu Jul 3 16:05:04 MDT 2014


The branch, master has been updated
       via  0c97b7e torture4: Make raw.lock.multilock fail after 20 seconds
       via  4205463 torture4: Adapt comment to code
       via  64346a1 s4: smbtorture: Add multi-lock test. Regression test for bug #10684.
       via  954401f s3: smbd: Locking - re-add pending lock records if we fail to acquire a lock (and the lock hasn't timed out).
       via  cc9de6e s3: smbd: Locking - treat lock timeout the same as any other error.
       via  12be57e s3: smbd: Locking - add and use utility function lock_timed_out().
       via  517fa80 s3: smbd: Locking - convert to using utility macro used elsewhere.
      from  4126f97 torture3: Fix bug 10687

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 0c97b7eb5359b95c0d51a3b5524e82e34243d2d1
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jul 3 10:05:55 2014 +0000

    torture4: Make raw.lock.multilock fail after 20 seconds
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Fri Jul  4 00:04:10 CEST 2014 on sn-devel-104

commit 4205463ef1815d6e86e1d1f1f57651ca30407469
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jul 3 10:05:39 2014 +0000

    torture4: Adapt comment to code
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 64346a134dac2bd023f7473202ca38d35ffd3c89
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jul 1 12:05:07 2014 -0700

    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 at samba.org>
    Reviewed-by: Volker Lendecke <Volker.Lendecke at SerNet.DE>

commit 954401f8b2b16b3e2ef9655e8ce94d657becce36
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Jul 2 20:51:24 2014 -0700

    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 <jra at samba.org>
    Reviewed-by: Volker Lendecke <Volker.Lendecke at SerNet.DE>

commit cc9de6eb091159a84228b988c49261c46c301233
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Jul 2 20:40:49 2014 -0700

    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 <jra at samba.org>
    Reviewed-by: Volker Lendecke <Volker.Lendecke at SerNet.DE>

commit 12be57ef3b2d1b670be7a83f29cd580938030015
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Jul 2 20:18:42 2014 -0700

    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 <jra at samba.org>
    Reviewed-by: Volker Lendecke <Volker.Lendecke at SerNet.DE>

commit 517fa80bd385c6adcfee03ea6b25599013ad88f5
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Jul 2 17:25:22 2014 -0700

    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 <jra at samba.org>
    Reviewed-by: Volker Lendecke <Volker.Lendecke at SerNet.DE>

-----------------------------------------------------------------------

Summary of changes:
 source3/smbd/blocking.c    |  195 +++++++++++++++++++++++++++++---------------
 source4/torture/raw/lock.c |   97 ++++++++++++++++++++++
 2 files changed, 227 insertions(+), 65 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c
index 5d198fc..fdb90a1 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.
 *****************************************************************************/
@@ -440,11 +459,10 @@ 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;
+	bool lock_timeout = lock_timed_out(blr);
 
 	data = discard_const_p(uint8_t, blr->req->buf)
 		+ ((large_file_format ? 20 : 10)*num_ulocks);
@@ -458,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
@@ -469,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,
@@ -480,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)) {
@@ -500,8 +551,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.
@@ -512,6 +562,14 @@ static bool process_lockingX(struct blocking_lock_record *blr)
 	}
 
 	/*
+	 * 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.
 	 */
 
@@ -532,6 +590,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,
@@ -544,10 +604,46 @@ 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)) {
 		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;
 		}
@@ -735,11 +831,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;
 	}
 
@@ -748,6 +843,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,65 +863,34 @@ 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);
-			continue;
-		}
-
 		/*
-		 * We couldn't get the locks for this record on the list.
-		 * If the time has expired, return a lock error.
+		 * Remove the pending lock we're waiting on.
+		 * If we need to keep waiting blocking_lock_record_process()
+		 * will re-add it.
 		 */
 
-		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.
-			 */
+		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 (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)));
+		if(!blocking_lock_record_process(blr)) {
+			DEBUG(10, ("still waiting for lock. BLR = %p\n", blr));
+			continue;
+		}
 
-				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: removing 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);
-		}
+		DLIST_REMOVE(sconn->smb1.locks.blocking_lock_queue, blr);
+		TALLOC_FREE(blr);
 	}
 
 	recalc_brl_timeout(sconn);
diff --git a/source4/torture/raw/lock.c b/source4/torture/raw/lock.c
index a31a4d0..e131e27 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, 120->129 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 = 20000;
+	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;
 }


-- 
Samba Shared Repository


More information about the samba-cvs mailing list