[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Tue Aug 21 00:34:02 UTC 2018


The branch, master has been updated
       via  ec3c37e torture: Demonstrate the invalid lock order panic
       via  51d5707 vfs_fruit: Fix a leak of "br_lck"
      from  9ee4d94 python: Fix print in dns_invalid.py

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


- Log -----------------------------------------------------------------
commit ec3c37ee53f21d8c0e80b1d3b3d7e95a4ac8e0bc
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Aug 6 14:35:15 2018 +0200

    torture: Demonstrate the invalid lock order panic
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13584
    
    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): Tue Aug 21 02:33:05 CEST 2018 on sn-devel-144

commit 51d57073798f76ec4f1261945e0ba779b2530009
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Aug 6 14:33:34 2018 +0200

    vfs_fruit: Fix a leak of "br_lck"
    
    Fix a panic if fruit_access_check detects a locking conflict.
    
    do_lock() returns a valid br_lck even in case of a locking conflict.
    Not free'ing it leads to a invalid lock order panic later, because
    "br_lck" corresponds to a dbwrap lock on brlock.tdb.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13584
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 source3/modules/vfs_fruit.c | 24 ++++++++----
 source4/torture/vfs/fruit.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 8 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 0784262..ebf0f98 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -2386,7 +2386,6 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 				   uint32_t deny_mode)
 {
 	NTSTATUS status = NT_STATUS_OK;
-	struct byte_range_lock *br_lck = NULL;
 	bool open_for_reading, open_for_writing, deny_read, deny_write;
 	off_t off;
 	bool have_read = false;
@@ -2444,6 +2443,8 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 
 		/* Set locks */
 		if ((access_mask & FILE_READ_DATA) && have_read) {
+			struct byte_range_lock *br_lck = NULL;
+
 			off = access_to_netatalk_brl(fork_type, FILE_READ_DATA);
 			br_lck = do_lock(
 				handle->conn->sconn->msg_ctx, fsp,
@@ -2451,13 +2452,16 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 				READ_LOCK, POSIX_LOCK, false,
 				&status, NULL);
 
+			TALLOC_FREE(br_lck);
+
 			if (!NT_STATUS_IS_OK(status))  {
 				return status;
 			}
-			TALLOC_FREE(br_lck);
 		}
 
 		if ((deny_mode & DENY_READ) && have_read) {
+			struct byte_range_lock *br_lck = NULL;
+
 			off = denymode_to_netatalk_brl(fork_type, DENY_READ);
 			br_lck = do_lock(
 				handle->conn->sconn->msg_ctx, fsp,
@@ -2465,10 +2469,11 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 				READ_LOCK, POSIX_LOCK, false,
 				&status, NULL);
 
+			TALLOC_FREE(br_lck);
+
 			if (!NT_STATUS_IS_OK(status)) {
 				return status;
 			}
-			TALLOC_FREE(br_lck);
 		}
 	}
 
@@ -2494,6 +2499,8 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 
 		/* Set locks */
 		if ((access_mask & FILE_WRITE_DATA) && have_read) {
+			struct byte_range_lock *br_lck = NULL;
+
 			off = access_to_netatalk_brl(fork_type, FILE_WRITE_DATA);
 			br_lck = do_lock(
 				handle->conn->sconn->msg_ctx, fsp,
@@ -2501,13 +2508,15 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 				READ_LOCK, POSIX_LOCK, false,
 				&status, NULL);
 
+			TALLOC_FREE(br_lck);
+
 			if (!NT_STATUS_IS_OK(status)) {
 				return status;
 			}
-			TALLOC_FREE(br_lck);
-
 		}
 		if ((deny_mode & DENY_WRITE) && have_read) {
+			struct byte_range_lock *br_lck = NULL;
+
 			off = denymode_to_netatalk_brl(fork_type, DENY_WRITE);
 			br_lck = do_lock(
 				handle->conn->sconn->msg_ctx, fsp,
@@ -2515,15 +2524,14 @@ static NTSTATUS fruit_check_access(vfs_handle_struct *handle,
 				READ_LOCK, POSIX_LOCK, false,
 				&status, NULL);
 
+			TALLOC_FREE(br_lck);
+
 			if (!NT_STATUS_IS_OK(status)) {
 				return status;
 			}
-			TALLOC_FREE(br_lck);
 		}
 	}
 
-	TALLOC_FREE(br_lck);
-
 	return status;
 }
 
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index 4c49a6b..fc1760e 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -4891,6 +4891,93 @@ done:
 	return ret;
 }
 
+static bool test_fruit_locking_conflict(struct torture_context *tctx,
+					struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx;
+	struct smb2_create create;
+	struct smb2_handle h;
+	struct smb2_lock lck;
+	struct smb2_lock_element el;
+	const char *fname = BASEDIR "\\locking_conflict.txt";
+	NTSTATUS status;
+	bool ret = false;
+
+	mem_ctx = talloc_new(tctx);
+	torture_assert_not_null(tctx, mem_ctx, "talloc_new failed");
+
+	/* clean slate ...*/
+	smb2_util_unlink(tree, fname);
+	smb2_deltree(tree, fname);
+	smb2_deltree(tree, BASEDIR);
+
+	status = torture_smb2_testdir(tree, BASEDIR, &h);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	smb2_util_close(tree, h);
+
+	create = (struct smb2_create) {
+		.in.desired_access = SEC_RIGHTS_FILE_READ,
+		.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+		.in.share_access =
+		NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE,
+		.in.create_disposition = NTCREATEX_DISP_CREATE,
+		.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS,
+		.in.fname = fname,
+	};
+
+	status = smb2_create(tree, mem_ctx, &create);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h = create.out.file.handle;
+
+	el = (struct smb2_lock_element) {
+		.offset = 0xfffffffffffffffc,
+		.length = 1,
+		.flags = SMB2_LOCK_FLAG_EXCLUSIVE,
+	};
+	lck = (struct smb2_lock) {
+		.in.lock_count = 1,
+		.in.file.handle = h,
+		.in.locks = &el,
+	};
+
+	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	el = (struct smb2_lock_element) {
+		.offset = 0,
+		.length = 0x7fffffffffffffff,
+		.flags = SMB2_LOCK_FLAG_EXCLUSIVE,
+	};
+	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	create = (struct smb2_create) {
+		.in.desired_access =
+		SEC_RIGHTS_FILE_READ|SEC_RIGHTS_FILE_WRITE,
+		.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_READ,
+		.in.create_disposition = NTCREATEX_DISP_OPEN,
+		.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS,
+		.in.fname = fname,
+	};
+
+	status = smb2_create(tree, mem_ctx, &create);
+	CHECK_STATUS(status, NT_STATUS_FILE_LOCK_CONFLICT);
+
+	{
+		struct smb2_close cl = {
+			.level = RAW_CLOSE_SMB2,
+			.in.file.handle = h,
+		};
+		smb2_close(tree, &cl);
+	}
+
+	ret = true;
+done:
+	return ret;
+}
+
 struct torture_suite *torture_vfs_fruit_netatalk(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite = torture_suite_create(
@@ -4900,6 +4987,8 @@ struct torture_suite *torture_vfs_fruit_netatalk(TALLOC_CTX *ctx)
 
 	torture_suite_add_1smb2_test(suite, "read netatalk metadata", test_read_netatalk_metadata);
 	torture_suite_add_1smb2_test(suite, "stream names with locally created xattr", test_stream_names_local);
+	torture_suite_add_1smb2_test(
+		suite, "locking conflict", test_fruit_locking_conflict);
 
 	return suite;
 }


-- 
Samba Shared Repository



More information about the samba-cvs mailing list