[PATCHSET] Don't break oplocks if we have DELETE_PENDING

Volker Lendecke Volker.Lendecke at SerNet.DE
Wed Oct 9 13:06:57 MDT 2013


Hi!

Attached find extended oplock.doc tests together with a fix
for Samba. The DELETE_ON_CLOSE test comes before oplock
breaks.

Please review&push!

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de

*****************************************************************
visit us on it-sa:IT security exhibitions in Nürnberg, Germany
October 8th - 10th 2013, hall 12, booth 333
free tickets available via code 270691 on: www.it-sa.de/gutschein
******************************************************************
-------------- next part --------------
From 22a370410cfadffa7a0f937f4c2bc8129b085744 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 19:00:57 -0700
Subject: [PATCH 1/3] torture: Extend the raw.oplock.doc1 test

If delete_on_close is set, there is no oplock break. Check that.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 selftest/knownfail           |    1 +
 source4/torture/raw/oplock.c |   28 +++++++++++++++++++++-------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 8b89f00..e349c63 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -138,6 +138,7 @@
 ^samba4.smb2.lock.*.multiple-unlock # bug 6959
 ^samba4.raw.sfileinfo.*.end-of-file\(.*\)$ # bug 6962
 ^samba4.raw.oplock.*.batch22 # bug 6963
+^samba4.raw.oplock.*.doc1
 ^samba4.raw.lock.*.zerobyteread # bug 6974
 ^samba4.smb2.lock.*.zerobyteread # bug 6974
 ^samba4.raw.streams.*.delete
diff --git a/source4/torture/raw/oplock.c b/source4/torture/raw/oplock.c
index c2e086a..b4aac11 100644
--- a/source4/torture/raw/oplock.c
+++ b/source4/torture/raw/oplock.c
@@ -3576,7 +3576,8 @@ static bool test_raw_oplock_stream1(struct torture_context *tctx,
 }
 
 static bool test_raw_oplock_doc(struct torture_context *tctx,
-				struct smbcli_state *cli)
+				struct smbcli_state *cli,
+				struct smbcli_state *cli2)
 {
 	const char *fname = BASEDIR "\\test_oplock_doc.dat";
 	NTSTATUS status;
@@ -3600,16 +3601,15 @@ static bool test_raw_oplock_doc(struct torture_context *tctx,
 	io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_ALL;
 	io.ntcreatex.in.alloc_size = 0;
 	io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL;
-	io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+	io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|NTCREATEX_SHARE_ACCESS_DELETE;
 	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN_IF;
-	io.ntcreatex.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
+	io.ntcreatex.in.create_options = 0;
 	io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
 	io.ntcreatex.in.security_flags = 0;
 	io.ntcreatex.in.fname = fname;
 
-	torture_comment(tctx, "open a delete-on-close file with a batch "
-			"oplock\n");
-	ZERO_STRUCT(break_info);
+	torture_comment(tctx, "open a file with a batch oplock\n");
 	io.ntcreatex.in.flags = NTCREATEX_FLAGS_EXTENDED |
 	    NTCREATEX_FLAGS_REQUEST_OPLOCK |
 	    NTCREATEX_FLAGS_REQUEST_BATCH_OPLOCK;
@@ -3619,6 +3619,20 @@ static bool test_raw_oplock_doc(struct torture_context *tctx,
 	fnum = io.ntcreatex.out.file.fnum;
 	CHECK_VAL(io.ntcreatex.out.oplock_level, BATCH_OPLOCK_RETURN);
 
+	torture_comment(tctx, "Set delete-on-close\n");
+	status = smbcli_nt_delete_on_close(cli->tree, fnum, true);
+	CHECK_STATUS(tctx, status, NT_STATUS_OK);
+
+	torture_comment(tctx, "2nd open should not break and get "
+			"DELETE_PENDING\n");
+	ZERO_STRUCT(break_info);
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+	io.ntcreatex.in.create_options = 0;
+	io.ntcreatex.in.access_mask = SEC_FILE_READ_DATA;
+	status = smb_raw_open(cli2->tree, tctx, &io);
+	CHECK_STATUS(tctx, status, NT_STATUS_DELETE_PENDING);
+	CHECK_VAL(break_info.count, 0);
+
 	smbcli_close(cli->tree, fnum);
 
 done:
@@ -4075,7 +4089,7 @@ struct torture_suite *torture_raw_oplock(TALLOC_CTX *mem_ctx)
 	torture_suite_add_2smb_test(suite, "batch25", test_raw_oplock_batch25);
 	torture_suite_add_2smb_test(suite, "batch26", test_raw_oplock_batch26);
 	torture_suite_add_2smb_test(suite, "stream1", test_raw_oplock_stream1);
-	torture_suite_add_1smb_test(suite, "doc1", test_raw_oplock_doc);
+	torture_suite_add_2smb_test(suite, "doc1", test_raw_oplock_doc);
 	torture_suite_add_2smb_test(suite, "brl1", test_raw_oplock_brl1);
 	torture_suite_add_1smb_test(suite, "brl2", test_raw_oplock_brl2);
 	torture_suite_add_1smb_test(suite, "brl3", test_raw_oplock_brl3);
-- 
1.7.9.5


From 5eb02a76faa4cde3d4fe24e3b79e0ec33fe51895 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 23:04:50 -0700
Subject: [PATCH 2/3] torture: Extend the smb2.oplock.doc1 test

If delete_on_close is set, there is no oplock break. Check that.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 selftest/knownfail            |    1 +
 source4/torture/smb2/oplock.c |   34 ++++++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index e349c63..1653cea 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -122,6 +122,7 @@
 ^samba4.smb2.rename.share_delete_no_delete_access\(.*\)$
 ^samba4.smb2.rename.no_share_delete_no_delete_access\(.*\)$
 ^samba4.smb2.rename.msword
+^samba4.smb2.oplock.doc
 ^samba4.smb2.compound.related3
 ^samba4.smb2.compound.compound-break
 ^samba4.winbind.struct.*.show_sequence     # Not yet working in winbind
diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c
index 4cf7c7d..5070d00 100644
--- a/source4/torture/smb2/oplock.c
+++ b/source4/torture/smb2/oplock.c
@@ -2896,16 +2896,19 @@ static bool test_raw_oplock_stream1(struct torture_context *tctx,
 	return ret;
 }
 
-static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree *tree)
+static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree *tree,
+				 struct smb2_tree *tree2)
 {
 	const char *fname = BASEDIR "\\test_oplock_doc.dat";
 	NTSTATUS status;
 	bool ret = true;
 	union smb_open io;
 	struct smb2_handle h, h1;
+	union smb_setfileinfo sfinfo;
 
 	status = torture_smb2_testdir(tree, BASEDIR, &h);
 	torture_assert_ntstatus_ok(tctx, status, "Error creating directory");
+	smb2_util_close(tree, h);
 
 	/* cleanup */
 	smb2_util_unlink(tree, fname);
@@ -2920,15 +2923,15 @@ static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree
 	io.smb2.in.desired_access = SEC_RIGHTS_FILE_ALL;
 	io.smb2.in.alloc_size = 0;
 	io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
-	io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+	io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE|NTCREATEX_SHARE_ACCESS_DELETE;
 	io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
-	io.smb2.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
+	io.smb2.in.create_options = 0;
 	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
 	io.smb2.in.security_flags = 0;
 	io.smb2.in.fname = fname;
 
-	torture_comment(tctx, "open a delete-on-close file with a batch "
-			"oplock\n");
+	torture_comment(tctx, "open a file with a batch oplock\n");
 	ZERO_STRUCT(break_info);
 	io.smb2.in.create_flags = NTCREATEX_FLAGS_EXTENDED;
 	io.smb2.in.oplock_level = SMB2_OPLOCK_LEVEL_BATCH;
@@ -2938,6 +2941,25 @@ static bool test_smb2_oplock_doc(struct torture_context *tctx, struct smb2_tree
 	h1 = io.smb2.out.file.handle;
 	CHECK_VAL(io.smb2.out.oplock_level, SMB2_OPLOCK_LEVEL_BATCH);
 
+	torture_comment(tctx, "Set delete on close\n");
+	ZERO_STRUCT(sfinfo);
+	sfinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION;
+	sfinfo.generic.in.file.handle = h1;
+	sfinfo.disposition_info.in.delete_on_close = 1;
+	status = smb2_setinfo_file(tree, &sfinfo);
+	torture_assert_ntstatus_ok(tctx, status, "Incorrect status");
+
+	torture_comment(tctx, "2nd open should not break and get "
+			"DELETE_PENDING\n");
+	ZERO_STRUCT(break_info);
+	io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
+	io.smb2.in.create_options = 0;
+	io.smb2.in.desired_access = SEC_FILE_READ_DATA;
+	status = smb2_create(tree2, tctx, &io.smb2);
+	torture_assert_ntstatus_equal(tctx, status, NT_STATUS_DELETE_PENDING,
+				      "Incorrect status");
+	CHECK_VAL(break_info.count, 0);
+
 	smb2_util_close(tree, h1);
 
 	smb2_util_unlink(tree, fname);
@@ -3412,7 +3434,7 @@ struct torture_suite *torture_smb2_oplocks_init(void)
 	torture_suite_add_2smb2_test(suite, "batch24", test_smb2_oplock_batch24);
 	torture_suite_add_1smb2_test(suite, "batch25", test_smb2_oplock_batch25);
 	torture_suite_add_2smb2_test(suite, "stream1", test_raw_oplock_stream1);
-	torture_suite_add_1smb2_test(suite, "doc", test_smb2_oplock_doc);
+	torture_suite_add_2smb2_test(suite, "doc", test_smb2_oplock_doc);
 	torture_suite_add_2smb2_test(suite, "brl1", test_smb2_oplock_brl1);
 	torture_suite_add_1smb2_test(suite, "brl2", test_smb2_oplock_brl2);
 	torture_suite_add_1smb2_test(suite, "brl3", test_smb2_oplock_brl3);
-- 
1.7.9.5


From 300e7628cfc3f0d92db43b492e43de5a7382d367 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 18:41:07 -0700
Subject: [PATCH 3/3] smbd: Fix the extended *.oplock.doc1 tests

We need to check for DELETE_PENDING before the first oplock break

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/open.c |   59 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5024c90..6255180 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1093,6 +1093,26 @@ bool is_stat_open(uint32 access_mask)
 		((access_mask & ~stat_open_bits) == 0));
 }
 
+static bool has_delete_on_close(struct share_mode_lock *lck,
+				uint32_t name_hash)
+{
+	struct share_mode_data *d = lck->data;
+	uint32_t i;
+
+	if (d->num_share_modes == 0) {
+		return false;
+	}
+	if (!is_delete_on_close_set(lck, name_hash)) {
+		return false;
+	}
+	for (i=0; i<d->num_share_modes; i++) {
+		if (!share_mode_stale_pid(d, i)) {
+			return true;
+		}
+	}
+	return false;
+}
+
 /****************************************************************************
  Deal with share modes
  Invarient: Share mode must be locked on entry and exit.
@@ -1113,25 +1133,6 @@ static NTSTATUS open_mode_check(connection_struct *conn,
 		return NT_STATUS_OK;
 	}
 
-	/* A delete on close prohibits everything */
-
-	if (is_delete_on_close_set(lck, name_hash)) {
-		/*
-		 * Check the delete on close token
-		 * is valid. It could have been left
-		 * after a server crash.
-		 */
-		for(i = 0; i < lck->data->num_share_modes; i++) {
-			if (!share_mode_stale_pid(lck->data, i)) {
-
-				*file_existed = true;
-
-				return NT_STATUS_DELETE_PENDING;
-			}
-		}
-		return NT_STATUS_OK;
-	}
-
 	if (is_stat_open(access_mask)) {
 		/* Stat open that doesn't trigger oplock breaks or share mode
 		 * checks... ! JRA. */
@@ -2416,6 +2417,12 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 			  &got_level2_oplock,
 			  &got_a_none_oplock);
 
+	if (has_delete_on_close(lck, fsp->name_hash)) {
+		TALLOC_FREE(lck);
+		fd_close(fsp);
+		return NT_STATUS_DELETE_PENDING;
+	}
+
 	/* First pass - send break only on batch oplocks. */
 	if ((req != NULL) &&
 	    delay_for_batch_oplocks(fsp,
@@ -2450,13 +2457,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		}
 	}
 
-	if (NT_STATUS_EQUAL(status, NT_STATUS_DELETE_PENDING)) {
-		/* DELETE_PENDING is not deferred for a second */
-		TALLOC_FREE(lck);
-		fd_close(fsp);
-		return status;
-	}
-
 	if (!NT_STATUS_IS_OK(status)) {
 		uint32 can_access_mask;
 		bool can_access = True;
@@ -3166,6 +3166,13 @@ static NTSTATUS open_directory(connection_struct *conn,
 		return NT_STATUS_SHARING_VIOLATION;
 	}
 
+	if (has_delete_on_close(lck, fsp->name_hash)) {
+		TALLOC_FREE(lck);
+		fd_close(fsp);
+		file_free(req, fsp);
+		return NT_STATUS_DELETE_PENDING;
+	}
+
 	status = open_mode_check(conn, lck, fsp->name_hash,
 				access_mask, share_access,
 				 create_options, &dir_existed);
-- 
1.7.9.5



More information about the samba-technical mailing list