Patchset: Fix a cleanup bug

Volker Lendecke Volker.Lendecke at SerNet.DE
Tue Sep 3 09:14:36 MDT 2013


On Tue, Sep 03, 2013 at 01:14:29AM +0200, Michael Adam wrote:
> FYI: I started to review the patchset,
> but need to finish tomorrow.

Thanks!

Attached find a new patchset that contains the one already
sent together with a few patches that are supposed to
simplify and clean up our open code a bit.

So if you have time, 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 b61eeabda3b457ca6e5d78d5070811cbb05f3b2e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 22 Aug 2013 08:49:07 +0000
Subject: [PATCH 01/25] smbd: Simplify find_oplock_types

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 53f8b8e..c28d2a3 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1237,19 +1237,20 @@ static void find_oplock_types(files_struct *fsp,
 	}
 
 	for (i=0; i<lck->data->num_share_modes; i++) {
-		if (!is_valid_share_mode_entry(&lck->data->share_modes[i])) {
+		struct share_mode_entry *e = &lck->data->share_modes[i];
+
+		if (!is_valid_share_mode_entry(e)) {
 			continue;
 		}
 
-		if (lck->data->share_modes[i].op_type == NO_OPLOCK &&
-				is_stat_open(lck->data->share_modes[i].access_mask)) {
+		if (e->op_type == NO_OPLOCK && is_stat_open(e->access_mask)) {
 			/* We ignore stat opens in the table - they
 			   always have NO_OPLOCK and never get or
 			   cause breaks. JRA. */
 			continue;
 		}
 
-		if (BATCH_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) {
+		if (BATCH_OPLOCK_TYPE(e->op_type)) {
 			/* batch - can only be one. */
 			if (share_mode_stale_pid(lck->data, i)) {
 				DEBUG(10, ("Found stale batch oplock\n"));
@@ -1258,10 +1259,10 @@ static void find_oplock_types(files_struct *fsp,
 			if (*pp_ex_or_batch || *pp_batch || *got_level2 || *got_no_oplock) {
 				smb_panic("Bad batch oplock entry.");
 			}
-			*pp_batch = &lck->data->share_modes[i];
+			*pp_batch = e;
 		}
 
-		if (EXCLUSIVE_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) {
+		if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
 			if (share_mode_stale_pid(lck->data, i)) {
 				DEBUG(10, ("Found stale duplicate oplock\n"));
 				continue;
@@ -1270,10 +1271,10 @@ static void find_oplock_types(files_struct *fsp,
 			if (*pp_ex_or_batch || *got_level2 || *got_no_oplock) {
 				smb_panic("Bad exclusive or batch oplock entry.");
 			}
-			*pp_ex_or_batch = &lck->data->share_modes[i];
+			*pp_ex_or_batch = e;
 		}
 
-		if (LEVEL_II_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) {
+		if (LEVEL_II_OPLOCK_TYPE(e->op_type)) {
 			if (*pp_batch || *pp_ex_or_batch) {
 				if (share_mode_stale_pid(lck->data, i)) {
 					DEBUG(10, ("Found stale LevelII "
@@ -1285,7 +1286,7 @@ static void find_oplock_types(files_struct *fsp,
 			*got_level2 = true;
 		}
 
-		if (lck->data->share_modes[i].op_type == NO_OPLOCK) {
+		if (e->op_type == NO_OPLOCK) {
 			if (*pp_batch || *pp_ex_or_batch) {
 				if (share_mode_stale_pid(lck->data, i)) {
 					DEBUG(10, ("Found stale NO_OPLOCK "
-- 
1.8.1.2


From 2a579832b96a584e974cb573832b4c152164f94c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 1 Sep 2013 11:07:19 +0200
Subject: [PATCH 02/25] smbd: Don't store in-memory only flags in locking.tdb

Hey, pidl knows the [skip] attribute ... :-)

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/librpc/idl/open_files.idl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
index fa87bc7..a2e386f 100644
--- a/source3/librpc/idl/open_files.idl
+++ b/source3/librpc/idl/open_files.idl
@@ -42,8 +42,8 @@ interface open_files
 		[size_is(num_delete_tokens)] delete_token delete_tokens[];
 		timespec old_write_time;
 		timespec changed_write_time;
-		uint8 fresh;
-		uint8 modified;
+		[skip] boolean8 fresh;
+		[skip] boolean8 modified;
 		[ignore] db_record *record;
 	} share_mode_data;
 
-- 
1.8.1.2


From 3d3b0d372b6f3d8242d8d875238829bdcea7a4da Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 30 Aug 2013 12:27:36 +0000
Subject: [PATCH 03/25] smbd: Rename parameter "i" to "idx"

We'll need "i" in a later checkin ... :-)

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 12 ++++++------
 source3/locking/proto.h   |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 7e65616..602c30d 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -634,24 +634,24 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
  * from d->share_modes. Modifies d->num_share_modes, watch out in
  * routines iterating over that array.
  */
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned i)
+bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
 {
 	struct share_mode_entry *e;
 
-	if (i > d->num_share_modes) {
+	if (idx > d->num_share_modes) {
 		DEBUG(1, ("Asking for index %u, only %u around\n",
-			  i, (unsigned)d->num_share_modes));
+			  idx, (unsigned)d->num_share_modes));
 		return false;
 	}
-	e = &d->share_modes[i];
+	e = &d->share_modes[idx];
 	if (serverid_exists(&e->pid)) {
 		DEBUG(10, ("PID %s (index %u out of %u) still exists\n",
-			   procid_str_static(&e->pid), i,
+			   procid_str_static(&e->pid), idx,
 			   (unsigned)d->num_share_modes));
 		return false;
 	}
 	DEBUG(10, ("PID %s (index %u out of %u) does not exist anymore\n",
-		   procid_str_static(&e->pid), i,
+		   procid_str_static(&e->pid), idx,
 		   (unsigned)d->num_share_modes));
 	*e = d->share_modes[d->num_share_modes-1];
 	d->num_share_modes -= 1;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index adb30b7..93fbea5 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -170,7 +170,7 @@ void get_file_infos(struct file_id id,
 		    bool *delete_on_close,
 		    struct timespec *write_time);
 bool is_valid_share_mode_entry(const struct share_mode_entry *e);
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned i);
+bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx);
 void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
 		    uid_t uid, uint64_t mid, uint16 op_type);
 bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp);
-- 
1.8.1.2


From 927e34e63f488ed714ff7da6963a1f3d8d58b957 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 30 Aug 2013 12:49:43 +0000
Subject: [PATCH 04/25] smbd: Fix flawed share_mode_stale_pid API

The comment for this routine said:

> Modifies d->num_share_modes, watch out in routines iterating over
> that array.

Well, it turns out that *every* caller of this API got it wrong. So I
think it's better to change the routine.

This leaves the array untouched while iterating but filters out the
deleted ones while saving them back to disk.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/librpc/idl/open_files.idl |  6 ++++++
 source3/locking/locking.c         | 35 ++++++++++++++++++++++++++---------
 source3/locking/share_mode_lock.c | 24 ++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
index a2e386f..686bc02 100644
--- a/source3/librpc/idl/open_files.idl
+++ b/source3/librpc/idl/open_files.idl
@@ -23,6 +23,12 @@ interface open_files
 		uint32		uid;
 		uint16		flags;
 		uint32		name_hash;
+
+		/*
+		 * In-memory flag indicating a non-existing pid. We don't want
+		 * to store this share_mode_entry on disk.
+		 */
+		[skip] boolean8	stale;
 	} share_mode_entry;
 
 	typedef [public] struct {
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 602c30d..5090082 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -617,6 +617,10 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
 {
 	int num_props = 0;
 
+	if (e->stale) {
+		return false;
+	}
+
 	num_props += ((e->op_type == NO_OPLOCK) ? 1 : 0);
 	num_props += (EXCLUSIVE_OPLOCK_TYPE(e->op_type) ? 1 : 0);
 	num_props += (LEVEL_II_OPLOCK_TYPE(e->op_type) ? 1 : 0);
@@ -630,9 +634,7 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
 /*
  * In case d->share_modes[i] conflicts with something or otherwise is
  * being used, we need to make sure the corresponding process still
- * exists. This routine checks it and potentially removes the entry
- * from d->share_modes. Modifies d->num_share_modes, watch out in
- * routines iterating over that array.
+ * exists.
  */
 bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
 {
@@ -653,17 +655,32 @@ bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
 	DEBUG(10, ("PID %s (index %u out of %u) does not exist anymore\n",
 		   procid_str_static(&e->pid), idx,
 		   (unsigned)d->num_share_modes));
-	*e = d->share_modes[d->num_share_modes-1];
-	d->num_share_modes -= 1;
 
-	if (d->num_share_modes == 0 &&
-	    d->num_delete_tokens) {
+	e->stale = true;
+
+	if (d->num_delete_tokens != 0) {
+		uint32_t i, num_stale;
+
 		/*
 		 * We cannot have any delete tokens
 		 * if there are no valid share modes.
 		 */
-		TALLOC_FREE(d->delete_tokens);
-		d->num_delete_tokens = 0;
+
+		num_stale = 0;
+
+		for (i=0; i<d->num_share_modes; i++) {
+			if (d->share_modes[i].stale) {
+				num_stale += 1;
+			}
+		}
+
+		if (num_stale == d->num_share_modes) {
+			/*
+			 * No non-stale share mode found
+			 */
+			TALLOC_FREE(d->delete_tokens);
+			d->num_delete_tokens = 0;
+		}
 	}
 
 	d->modified = true;
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 0693cf5..342f910 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -121,6 +121,7 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 {
 	struct share_mode_data *d;
 	enum ndr_err_code ndr_err;
+	uint32_t i;
 	DATA_BLOB blob;
 
 	d = talloc(mem_ctx, struct share_mode_data);
@@ -140,6 +141,14 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 		goto fail;
 	}
 
+	/*
+	 * Initialize the values that are [skip] in the idl. The NDR code does
+	 * not initialize them.
+	 */
+
+	for (i=0; i<d->num_share_modes; i++) {
+		d->share_modes[i].stale = false;
+	}
 	d->modified = false;
 	d->fresh = false;
 
@@ -162,12 +171,27 @@ static TDB_DATA unparse_share_modes(struct share_mode_data *d)
 {
 	DATA_BLOB blob;
 	enum ndr_err_code ndr_err;
+	uint32_t i;
 
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10, ("unparse_share_modes:\n"));
 		NDR_PRINT_DEBUG(share_mode_data, d);
 	}
 
+	i = 0;
+	while (i < d->num_share_modes) {
+		if (d->share_modes[i].stale) {
+			/*
+			 * Remove the stale entries before storing
+			 */
+			struct share_mode_entry *m = d->share_modes;
+			m[i] = m[d->num_share_modes-1];
+			d->num_share_modes -= 1;
+		} else {
+			i += 1;
+		}
+	}
+
 	if (d->num_share_modes == 0) {
 		DEBUG(10, ("No used share mode found\n"));
 		return make_tdb_data(NULL, 0);
-- 
1.8.1.2


From 2a8a3c9ae4d4902deb0a91d03a26f2286462d65f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 1 Sep 2013 18:54:59 +0200
Subject: [PATCH 05/25] torture3: Trigger a nasty cleanup bug in smbd

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py      |  1 +
 source3/torture/proto.h        |  1 +
 source3/torture/test_cleanup.c | 70 ++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c      |  1 +
 4 files changed, 73 insertions(+)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index afab687..63f119f 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -63,6 +63,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7"
         "SMB2-SESSION-REAUTH", "SMB2-SESSION-RECONNECT",
         "CLEANUP1",
         "CLEANUP2",
+        "CLEANUP4",
         "BAD-NBT-SESSION"]
 
 for t in tests:
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index f6453fd..d430aef 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -104,6 +104,7 @@ bool run_local_sprintf_append(int dummy);
 bool run_cleanup1(int dummy);
 bool run_cleanup2(int dummy);
 bool run_cleanup3(int dummy);
+bool run_cleanup4(int dummy);
 bool run_ctdb_conn(int dummy);
 bool run_msg_test(int dummy);
 bool run_notify_bench2(int dummy);
diff --git a/source3/torture/test_cleanup.c b/source3/torture/test_cleanup.c
index d9dce40..319a55f 100644
--- a/source3/torture/test_cleanup.c
+++ b/source3/torture/test_cleanup.c
@@ -329,3 +329,73 @@ bool run_cleanup3(int dummy)
 
 	return true;
 }
+
+bool run_cleanup4(int dummy)
+{
+	struct cli_state *cli1, *cli2;
+	const char *fname = "\\cleanup4";
+	uint16_t fnum1, fnum2;
+	NTSTATUS status;
+
+	printf("CLEANUP4: Checking that a conflicting share mode is cleaned "
+	       "up\n");
+
+	if (!torture_open_connection(&cli1, 0)) {
+		return false;
+	}
+	if (!torture_open_connection(&cli2, 0)) {
+		return false;
+	}
+
+	status = cli_ntcreate(
+		cli1, fname, 0,
+		FILE_GENERIC_READ|DELETE_ACCESS,
+		FILE_ATTRIBUTE_NORMAL,
+		FILE_SHARE_READ|FILE_SHARE_DELETE,
+		FILE_OVERWRITE_IF, 0, 0, &fnum1);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("creating file failed: %s\n",
+		       nt_errstr(status));
+		return false;
+	}
+
+	status = cli_ntcreate(
+		cli2, fname, 0,
+		FILE_GENERIC_READ|DELETE_ACCESS,
+		FILE_ATTRIBUTE_NORMAL,
+		FILE_SHARE_READ|FILE_SHARE_DELETE,
+		FILE_OPEN, 0, 0, &fnum2);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("opening file 1st time failed: %s\n",
+		       nt_errstr(status));
+		return false;
+	}
+
+	status = smbXcli_conn_samba_suicide(cli1->conn, 1);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("smbXcli_conn_samba_suicide failed: %s\n",
+		       nt_errstr(status));
+		return false;
+	}
+
+	/*
+	 * The next open will conflict with both opens above. The first open
+	 * above will be correctly cleaned up. A bug in smbd iterating over
+	 * the share mode array made it skip the share conflict check for the
+	 * second open. Trigger this bug.
+	 */
+
+	status = cli_ntcreate(
+		cli2, fname, 0,
+		FILE_GENERIC_WRITE|DELETE_ACCESS,
+		FILE_ATTRIBUTE_NORMAL,
+		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
+		FILE_OPEN, 0, 0, &fnum2);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION)) {
+		printf("opening file 2nd time returned: %s\n",
+		       nt_errstr(status));
+		return false;
+	}
+
+	return true;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 359baa9..2d1a107 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -9554,6 +9554,7 @@ static struct {
 	{ "CLEANUP1", run_cleanup1 },
 	{ "CLEANUP2", run_cleanup2 },
 	{ "CLEANUP3", run_cleanup3 },
+	{ "CLEANUP4", run_cleanup4 },
 	{ "LOCAL-SUBSTITUTE", run_local_substitute, 0},
 	{ "LOCAL-GENCACHE", run_local_gencache, 0},
 	{ "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0},
-- 
1.8.1.2


From 5cd295033e3884a9699fc2450068f55aed028d3a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 14:38:47 +0000
Subject: [PATCH 06/25] smbd: Simplify find_share_mode_entry

There's no point checking the validity of the "entry" argument more
than once

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 5090082..1567f8f 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -751,10 +751,13 @@ static struct share_mode_entry *find_share_mode_entry(struct share_mode_data *d,
 {
 	int i;
 
+	if (!is_valid_share_mode_entry(entry)) {
+		return NULL;
+	}
+
 	for (i=0; i<d->num_share_modes; i++) {
 		struct share_mode_entry *e = &d->share_modes[i];
-		if (is_valid_share_mode_entry(entry) &&
-		    is_valid_share_mode_entry(e) &&
+		if (is_valid_share_mode_entry(e) &&
 		    share_modes_identical(e, entry)) {
 			return e;
 		}
-- 
1.8.1.2


From d6ba0d1cf5a6f2c4bf18db0733e5fcca4e9d5d8a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 14:40:35 +0000
Subject: [PATCH 07/25] smbd: Apply some const to share_modes_identical

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 1567f8f..5343ba4 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -733,8 +733,8 @@ void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
  not automatically a logic error if they are identical. JRA.)
 ********************************************************************/
 
-static bool share_modes_identical(struct share_mode_entry *e1,
-				  struct share_mode_entry *e2)
+static bool share_modes_identical(const struct share_mode_entry *e1,
+				  const struct share_mode_entry *e2)
 {
 	/* We used to check for e1->share_access == e2->share_access here
 	   as well as the other fields but 2 different DOS or FCB opens
-- 
1.8.1.2


From 434aa9d09e23ec2c830511823bd23da3bc95523c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 14:40:58 +0000
Subject: [PATCH 08/25] smbd: Apply some const to find_share_mode_entry

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 5343ba4..ad56cb1 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -746,8 +746,8 @@ static bool share_modes_identical(const struct share_mode_entry *e1,
 		e1->share_file_id == e2->share_file_id );
 }
 
-static struct share_mode_entry *find_share_mode_entry(struct share_mode_data *d,
-						      struct share_mode_entry *entry)
+static struct share_mode_entry *find_share_mode_entry(
+	struct share_mode_data *d, const struct share_mode_entry *entry)
 {
 	int i;
 
-- 
1.8.1.2


From 768332ea3ad6cd9e25e431f89a4d807c9a3ed4ea Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 14:44:45 +0000
Subject: [PATCH 09/25] smbd: Remove unused should_notify_deferred_opens

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/oplock.c | 10 ----------
 source3/smbd/proto.h  |  1 -
 2 files changed, 11 deletions(-)

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 21792bd..9facbec 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -209,16 +209,6 @@ bool downgrade_oplock(files_struct *fsp)
 	return ret;
 }
 
-/*
- * Some kernel oplock implementations handle the notification themselves.
- */
-bool should_notify_deferred_opens(struct smbd_server_connection *sconn)
-{
-	struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
-	return !(koplocks &&
-		(koplocks->flags & KOPLOCKS_DEFERRED_OPEN_NOTIFICATION));
-}
-
 /****************************************************************************
  Set up an oplock break message.
 ****************************************************************************/
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index f572c35..d9b9d4d 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -663,7 +663,6 @@ NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type);
 void release_file_oplock(files_struct *fsp);
 bool remove_oplock(files_struct *fsp);
 bool downgrade_oplock(files_struct *fsp);
-bool should_notify_deferred_opens(struct smbd_server_connection *sconn);
 void contend_level2_oplocks_begin(files_struct *fsp,
 				  enum level2_contention_type type);
 void contend_level2_oplocks_end(files_struct *fsp,
-- 
1.8.1.2


From 698ce5e6faf40cabd2c873af8605af0bf9c868f7 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 09:23:39 +0000
Subject: [PATCH 10/25] smbd: Remove two confusing TALLOC_FREE calls

We don't have lck allocated yet at these points. Remove the TALLOC_FREE
calls that triggered me looking for the get_share_mode_lock calls.

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c28d2a3..27e2009 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2348,7 +2348,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
 			schedule_async_open(request_time, req);
 		}
-		TALLOC_FREE(lck);
 		return fsp_open;
 	}
 
@@ -2362,7 +2361,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		 * just fail the open to prevent creating any problems
 		 * in the open file db having the wrong dev/ino key.
 		 */
-		TALLOC_FREE(lck);
 		fd_close(fsp);
 		DEBUG(1,("open_file_ntcreate: file %s - dev/ino mismatch. "
 			"Old (dev=0x%llu, ino =0x%llu). "
-- 
1.8.1.2


From dba7fd05850b34574c5dbfefe6d8749aec37e8fb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 29 Aug 2013 14:53:01 +0000
Subject: [PATCH 11/25] smbd: Fix a const warning

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/trans2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index aaf0e62..75c2aa4 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -3078,7 +3078,7 @@ NTSTATUS smbd_do_qfsinfo(connection_struct *conn,
 	const char *vname = volume_label(talloc_tos(), SNUM(conn));
 	int snum = SNUM(conn);
 	char *fstype = lp_fstype(talloc_tos(), SNUM(conn));
-	char *filename = NULL;
+	const char *filename = NULL;
 	uint32 additional_flags = 0;
 	struct smb_filename smb_fname;
 	SMB_STRUCT_STAT st;
-- 
1.8.1.2


From 13b39afdcaffe9eb261ed0adef96ca1819c12ea9 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 11:51:48 +0000
Subject: [PATCH 12/25] lib: Apply some const to pull_file_id_24

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/file_id.c | 2 +-
 source3/lib/file_id.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/lib/file_id.c b/source3/lib/file_id.c
index ba4b3a3..f8295ce 100644
--- a/source3/lib/file_id.c
+++ b/source3/lib/file_id.c
@@ -80,7 +80,7 @@ void push_file_id_24(char *buf, const struct file_id *id)
 /*
   pull a 24 byte version of a file id from a buffer
  */
-void pull_file_id_24(char *buf, struct file_id *id)
+void pull_file_id_24(const char *buf, struct file_id *id)
 {
 	ZERO_STRUCTP(id);
 	id->devid  = IVAL(buf,  0);
diff --git a/source3/lib/file_id.h b/source3/lib/file_id.h
index 2ca8390..6fa9840 100644
--- a/source3/lib/file_id.h
+++ b/source3/lib/file_id.h
@@ -34,4 +34,4 @@ const char *file_id_string_tos(const struct file_id *id);
 const char *file_id_string(TALLOC_CTX *mem_ctx, const struct file_id *id);
 void push_file_id_16(char *buf, const struct file_id *id);
 void push_file_id_24(char *buf, const struct file_id *id);
-void pull_file_id_24(char *buf, struct file_id *id);
+void pull_file_id_24(const char *buf, struct file_id *id);
-- 
1.8.1.2


From cc08c551288e61f6ddf6efb5d106bdf6de327485 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 11:55:27 +0000
Subject: [PATCH 13/25] smbd: Apply some const to message_to_share_mode_entry

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/oplock.c | 2 +-
 source3/smbd/proto.h  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 9facbec..6bd6475 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -816,7 +816,7 @@ void share_mode_entry_to_message(char *msg, const struct share_mode_entry *e)
  De-linearize an internal oplock break message to a share mode entry struct.
 ****************************************************************************/
 
-void message_to_share_mode_entry(struct share_mode_entry *e, char *msg)
+void message_to_share_mode_entry(struct share_mode_entry *e, const char *msg)
 {
 	e->pid.pid = (pid_t)IVAL(msg,OP_BREAK_MSG_PID_OFFSET);
 	e->op_mid = BVAL(msg,OP_BREAK_MSG_MID_OFFSET);
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index d9b9d4d..8b6987e 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -672,7 +672,7 @@ void smbd_contend_level2_oplocks_begin(files_struct *fsp,
 void smbd_contend_level2_oplocks_end(files_struct *fsp,
 				enum level2_contention_type type);
 void share_mode_entry_to_message(char *msg, const struct share_mode_entry *e);
-void message_to_share_mode_entry(struct share_mode_entry *e, char *msg);
+void message_to_share_mode_entry(struct share_mode_entry *e, const char *msg);
 bool init_oplocks(struct smbd_server_connection *sconn);
 void init_kernel_oplocks(struct smbd_server_connection *sconn);
 
-- 
1.8.1.2


From 7fcf87f2af977af32715bd42cb45ba76966d2770 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 09:40:46 +0000
Subject: [PATCH 14/25] smbd: Remove a silly "? true : false"

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/open.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 27e2009..920c3fc 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1363,7 +1363,7 @@ static bool file_has_brlocks(files_struct *fsp)
 	if (!br_lck)
 		return false;
 
-	return br_lck->num_locks > 0 ? true : false;
+	return (br_lck->num_locks > 0);
 }
 
 static void grant_fsp_oplock_type(files_struct *fsp,
-- 
1.8.1.2


From 597dff2bb1b5a75705d859c24d8122e800201f97 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 09:06:13 +0000
Subject: [PATCH 15/25] smbd: Slightly simplify send_break_message

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 920c3fc..e699a92 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1194,8 +1194,7 @@ static NTSTATUS send_break_message(files_struct *fsp,
 
 	status = messaging_send_buf(fsp->conn->sconn->msg_ctx, exclusive->pid,
 				    MSG_SMB_BREAK_REQUEST,
-				    (uint8 *)msg,
-				    MSG_SMB_SHARE_MODE_ENTRY_SIZE);
+				    (uint8 *)msg, sizeof(msg));
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(3, ("Could not send oplock break message: %s\n",
 			  nt_errstr(status)));
-- 
1.8.1.2


From fe5288b1ecb7cc2e88db7bcb000f9ace2f9e46ab Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 09:08:04 +0000
Subject: [PATCH 16/25] smbd: Slightly simplify do_break_to_none

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/oplock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 6bd6475..8253fd9 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -750,8 +750,7 @@ static void do_break_to_none(struct tevent_context *ctx,
 			messaging_send_buf(state->sconn->msg_ctx,
 					share_entry->pid,
 					MSG_SMB_ASYNC_LEVEL2_BREAK,
-					(uint8 *)msg,
-					MSG_SMB_SHARE_MODE_ENTRY_SIZE);
+					(uint8 *)msg, sizeof(msg));
 		}
 	}
 
-- 
1.8.1.2


From 6fd557047389e02c04ab84f49e2010e5412a5161 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 09:09:16 +0000
Subject: [PATCH 17/25] smbd: Slightly simplify enum_file_close_fn

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index 1af350a..8f3a4cf 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -2480,8 +2480,7 @@ static void enum_file_close_fn( const struct share_mode_entry *e,
 	state->r->out.result = ntstatus_to_werror(
 		messaging_send_buf(state->msg_ctx,
 				e->pid, MSG_SMB_CLOSE_FILE,
-				(uint8 *)msg,
-				MSG_SMB_SHARE_MODE_ENTRY_SIZE));
+				(uint8 *)msg, sizeof(msg)));
 }
 
 /********************************************************************
-- 
1.8.1.2


From eb54e4931b16371b78d29d5673e57477d0c818fd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 20 Aug 2013 11:58:15 +0000
Subject: [PATCH 18/25] smbd: Unify delay_for_*_oplocks

This is the same code in both routines

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index e699a92..5706db4 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1299,19 +1299,19 @@ static void find_oplock_types(files_struct *fsp,
 	}
 }
 
-static bool delay_for_batch_oplocks(files_struct *fsp,
-					uint64_t mid,
-					int oplock_request,
-					struct share_mode_entry *batch_entry)
+static bool delay_for_oplock(files_struct *fsp,
+			     uint64_t mid,
+			     int oplock_request,
+			     struct share_mode_entry *entry)
 {
 	if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
 		return false;
 	}
-	if (batch_entry == NULL) {
+	if (entry == NULL) {
 		return false;
 	}
 
-	if (server_id_is_disconnected(&batch_entry->pid)) {
+	if (server_id_is_disconnected(&entry->pid)) {
 		/*
 		 * TODO: clean up.
 		 * This could be achieved by sending a break message
@@ -1324,33 +1324,7 @@ static bool delay_for_batch_oplocks(files_struct *fsp,
 		return false;
 	}
 
-	/* Found a batch oplock */
-	send_break_message(fsp, batch_entry, mid, oplock_request);
-	return true;
-}
-
-static bool delay_for_exclusive_oplocks(files_struct *fsp,
-					uint64_t mid,
-					int oplock_request,
-					struct share_mode_entry *ex_entry)
-{
-	if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
-		return false;
-	}
-	if (ex_entry == NULL) {
-		return false;
-	}
-
-	if (server_id_is_disconnected(&ex_entry->pid)) {
-		/*
-		 * since only durable handles can get disconnected,
-		 * and we can only get durable handles with batch oplocks,
-		 * this should actually never be reached...
-		 */
-		return false;
-	}
-
-	send_break_message(fsp, ex_entry, mid, oplock_request);
+	send_break_message(fsp, entry, mid, oplock_request);
 	return true;
 }
 
@@ -2319,9 +2293,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		find_oplock_types(fsp, 0, lck, &batch_entry, &exclusive_entry,
 				  &got_level2_oplock, &got_a_none_oplock);
 
-		if (delay_for_batch_oplocks(fsp, req->mid, 0, batch_entry) ||
-		    delay_for_exclusive_oplocks(fsp, req->mid, 0,
-						exclusive_entry)) {
+		if (delay_for_oplock(fsp, req->mid, 0, batch_entry) ||
+		    delay_for_oplock(fsp, req->mid, 0, exclusive_entry)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			DEBUG(10, ("Sent oplock break request to kernel "
@@ -2415,10 +2388,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 	/* First pass - send break only on batch oplocks. */
 	if ((req != NULL) &&
-	    delay_for_batch_oplocks(fsp,
-				    req->mid,
-				    oplock_request,
-				    batch_entry)) {
+	    delay_for_oplock(fsp, req->mid, oplock_request,
+			     batch_entry)) {
 		schedule_defer_open(lck, request_time, req);
 		TALLOC_FREE(lck);
 		fd_close(fsp);
@@ -2435,11 +2406,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		/* Second pass - send break for both batch or
 		 * exclusive oplocks. */
 		if ((req != NULL) &&
-		    delay_for_exclusive_oplocks(
-			    fsp,
-			    req->mid,
-			    oplock_request,
-			    exclusive_entry)) {
+		    delay_for_oplock(fsp, req->mid, oplock_request,
+				     exclusive_entry)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-- 
1.8.1.2


From 13ba0af9dd8746b2110cf3fa3bd0230c0f478a0f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 11:37:57 +0000
Subject: [PATCH 19/25] smbd: Factor out remove_stale_share_mode_entries

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c         | 16 ++++++++++++++++
 source3/locking/proto.h           |  1 +
 source3/locking/share_mode_lock.c | 15 +--------------
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index ad56cb1..6a8dc6b 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -687,6 +687,22 @@ bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
 	return true;
 }
 
+void remove_stale_share_mode_entries(struct share_mode_data *d)
+{
+	uint32_t i;
+
+	i = 0;
+	while (i < d->num_share_modes) {
+		if (d->share_modes[i].stale) {
+			struct share_mode_entry *m = d->share_modes;
+			m[i] = m[d->num_share_modes-1];
+			d->num_share_modes -= 1;
+		} else {
+			i += 1;
+		}
+	}
+}
+
 /*******************************************************************
  Fill a share mode entry.
 ********************************************************************/
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 93fbea5..4cb3626 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -171,6 +171,7 @@ void get_file_infos(struct file_id id,
 		    struct timespec *write_time);
 bool is_valid_share_mode_entry(const struct share_mode_entry *e);
 bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx);
+void remove_stale_share_mode_entries(struct share_mode_data *d);
 void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
 		    uid_t uid, uint64_t mid, uint16 op_type);
 bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 342f910..5d7a08c 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -171,26 +171,13 @@ static TDB_DATA unparse_share_modes(struct share_mode_data *d)
 {
 	DATA_BLOB blob;
 	enum ndr_err_code ndr_err;
-	uint32_t i;
 
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10, ("unparse_share_modes:\n"));
 		NDR_PRINT_DEBUG(share_mode_data, d);
 	}
 
-	i = 0;
-	while (i < d->num_share_modes) {
-		if (d->share_modes[i].stale) {
-			/*
-			 * Remove the stale entries before storing
-			 */
-			struct share_mode_entry *m = d->share_modes;
-			m[i] = m[d->num_share_modes-1];
-			d->num_share_modes -= 1;
-		} else {
-			i += 1;
-		}
-	}
+	remove_stale_share_mode_entries(d);
 
 	if (d->num_share_modes == 0) {
 		DEBUG(10, ("No used share mode found\n"));
-- 
1.8.1.2


From 245bcdfb47f7925b78d97d55d54cebce398f5514 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 12:33:40 +0000
Subject: [PATCH 20/25] smbd: Further simplify find_oplock_types a bit

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5706db4..5f069d8 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1220,6 +1220,7 @@ static void find_oplock_types(files_struct *fsp,
 				bool *got_level2,
 				bool *got_no_oplock)
 {
+	struct share_mode_data *d = lck->data;
 	int i;
 
 	*pp_batch = NULL;
@@ -1235,8 +1236,8 @@ static void find_oplock_types(files_struct *fsp,
 		return;
 	}
 
-	for (i=0; i<lck->data->num_share_modes; i++) {
-		struct share_mode_entry *e = &lck->data->share_modes[i];
+	for (i=0; i<d->num_share_modes; i++) {
+		struct share_mode_entry *e = &d->share_modes[i];
 
 		if (!is_valid_share_mode_entry(e)) {
 			continue;
@@ -1251,7 +1252,7 @@ static void find_oplock_types(files_struct *fsp,
 
 		if (BATCH_OPLOCK_TYPE(e->op_type)) {
 			/* batch - can only be one. */
-			if (share_mode_stale_pid(lck->data, i)) {
+			if (share_mode_stale_pid(d, i)) {
 				DEBUG(10, ("Found stale batch oplock\n"));
 				continue;
 			}
@@ -1262,7 +1263,7 @@ static void find_oplock_types(files_struct *fsp,
 		}
 
 		if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
-			if (share_mode_stale_pid(lck->data, i)) {
+			if (share_mode_stale_pid(d, i)) {
 				DEBUG(10, ("Found stale duplicate oplock\n"));
 				continue;
 			}
@@ -1275,7 +1276,7 @@ static void find_oplock_types(files_struct *fsp,
 
 		if (LEVEL_II_OPLOCK_TYPE(e->op_type)) {
 			if (*pp_batch || *pp_ex_or_batch) {
-				if (share_mode_stale_pid(lck->data, i)) {
+				if (share_mode_stale_pid(d, i)) {
 					DEBUG(10, ("Found stale LevelII "
 						   "oplock\n"));
 					continue;
@@ -1287,7 +1288,7 @@ static void find_oplock_types(files_struct *fsp,
 
 		if (e->op_type == NO_OPLOCK) {
 			if (*pp_batch || *pp_ex_or_batch) {
-				if (share_mode_stale_pid(lck->data, i)) {
+				if (share_mode_stale_pid(d, i)) {
 					DEBUG(10, ("Found stale NO_OPLOCK "
 						   "entry\n"));
 					continue;
-- 
1.8.1.2


From b983570cb758ae51a4f8afd79a4ed05181fddb09 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 2 Sep 2013 12:25:07 +0000
Subject: [PATCH 21/25] smbd: Reduce the complexity of open_file_ntcreate

This removes two variables in open_file_ntcreate based on the observation
that for exclusive and batch oplocks there can only be one entry. So
in these cases we don't need to keep pointers from find_oplock_types to
delay_for_oplocks. We can just reference the only share mode entry around.

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5f069d8..5bed852 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1212,28 +1212,27 @@ static NTSTATUS send_break_message(files_struct *fsp,
  * Do internal consistency checks on the share mode for a file.
  */
 
-static void find_oplock_types(files_struct *fsp,
-				int oplock_request,
-				const struct share_mode_lock *lck,
-				struct share_mode_entry **pp_batch,
-				struct share_mode_entry **pp_ex_or_batch,
-				bool *got_level2,
-				bool *got_no_oplock)
+static bool validate_oplock_types(files_struct *fsp,
+				  int oplock_request,
+				  struct share_mode_lock *lck,
+				  bool *p_level2,
+				  bool *p_no_oplock)
 {
 	struct share_mode_data *d = lck->data;
+	bool batch = false;
+	bool ex_or_batch = false;
+	bool level2 = false;
+	bool no_oplock = false;
 	int i;
 
-	*pp_batch = NULL;
-	*pp_ex_or_batch = NULL;
-	*got_level2 = false;
-	*got_no_oplock = false;
-
 	/* Ignore stat or internal opens, as is done in
 		delay_for_batch_oplocks() and
 		delay_for_exclusive_oplocks().
 	 */
 	if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
-		return;
+		*p_level2 = false;
+		*p_no_oplock = false;
+		return true;
 	}
 
 	for (i=0; i<d->num_share_modes; i++) {
@@ -1256,10 +1255,12 @@ static void find_oplock_types(files_struct *fsp,
 				DEBUG(10, ("Found stale batch oplock\n"));
 				continue;
 			}
-			if (*pp_ex_or_batch || *pp_batch || *got_level2 || *got_no_oplock) {
-				smb_panic("Bad batch oplock entry.");
+			if (ex_or_batch || batch || level2 || no_oplock) {
+				DEBUG(1, ("Bad batch oplock entry at "
+					  "index %d.", i));
+				return false;
 			}
-			*pp_batch = e;
+			batch = true;
 		}
 
 		if (EXCLUSIVE_OPLOCK_TYPE(e->op_type)) {
@@ -1268,47 +1269,74 @@ static void find_oplock_types(files_struct *fsp,
 				continue;
 			}
 			/* Exclusive or batch - can only be one. */
-			if (*pp_ex_or_batch || *got_level2 || *got_no_oplock) {
-				smb_panic("Bad exclusive or batch oplock entry.");
+			if (ex_or_batch || level2 || no_oplock) {
+				DEBUG(1, ("Bad exclusive or batch oplock "
+					  "entry at index %d.\n", i));
+				return false;
 			}
-			*pp_ex_or_batch = e;
+			ex_or_batch = true;
 		}
 
 		if (LEVEL_II_OPLOCK_TYPE(e->op_type)) {
-			if (*pp_batch || *pp_ex_or_batch) {
+			if (batch || ex_or_batch) {
 				if (share_mode_stale_pid(d, i)) {
 					DEBUG(10, ("Found stale LevelII "
 						   "oplock\n"));
 					continue;
 				}
-				smb_panic("Bad levelII oplock entry.");
+				DEBUG(1, ("Bad levelII oplock entry at "
+					  "index %d.", i));
+				return false;
 			}
-			*got_level2 = true;
+			level2 = true;
 		}
 
 		if (e->op_type == NO_OPLOCK) {
-			if (*pp_batch || *pp_ex_or_batch) {
+			if (batch || ex_or_batch) {
 				if (share_mode_stale_pid(d, i)) {
 					DEBUG(10, ("Found stale NO_OPLOCK "
 						   "entry\n"));
 					continue;
 				}
-				smb_panic("Bad no oplock entry.");
+				DEBUG(1, ("Bad no oplock entry at "
+					  "index %d.\n", i));
+				return false;
 			}
-			*got_no_oplock = true;
+			no_oplock = true;
 		}
 	}
+
+	remove_stale_share_mode_entries(d);
+
+	if ((batch || ex_or_batch) && (d->num_share_modes != 1)) {
+		DEBUG(1, ("got batch (%d) or ex (%d) non-exclusively (%d)\n",
+			  (int)batch, (int)ex_or_batch,
+			  (int)d->num_share_modes));
+		return false;
+	}
+
+	*p_level2 = level2;
+	*p_no_oplock = no_oplock;
+	return true;
 }
 
 static bool delay_for_oplock(files_struct *fsp,
 			     uint64_t mid,
 			     int oplock_request,
-			     struct share_mode_entry *entry)
+			     struct share_mode_lock *lck,
+			     int delay_for_type)
 {
+	struct share_mode_entry *entry;
+
 	if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
 		return false;
 	}
-	if (entry == NULL) {
+	if (lck->data->num_share_modes != 1) {
+		return false;
+	}
+	entry = &lck->data->share_modes[0];
+
+	if (!(entry->op_type & delay_for_type)) {
 		return false;
 	}
 
@@ -1947,8 +1975,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	NTSTATUS status;
 	char *parent_dir;
 	SMB_STRUCT_STAT saved_stat = smb_fname->st;
-	struct share_mode_entry *batch_entry = NULL;
-	struct share_mode_entry *exclusive_entry = NULL;
 	bool got_level2_oplock = false;
 	bool got_a_none_oplock = false;
 	struct timespec old_write_time;
@@ -2291,11 +2317,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 
-		find_oplock_types(fsp, 0, lck, &batch_entry, &exclusive_entry,
-				  &got_level2_oplock, &got_a_none_oplock);
+		if (!validate_oplock_types(fsp, 0, lck, &got_level2_oplock,
+					   &got_a_none_oplock)) {
+			smb_panic("validate_oplock_types failed");
+		}
 
-		if (delay_for_oplock(fsp, req->mid, 0, batch_entry) ||
-		    delay_for_oplock(fsp, req->mid, 0, exclusive_entry)) {
+		if (delay_for_oplock(fsp, req->mid, 0, lck,
+				     BATCH_OPLOCK|EXCLUSIVE_OPLOCK)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			DEBUG(10, ("Sent oplock break request to kernel "
@@ -2379,18 +2407,15 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	}
 
 	/* Get the types we need to examine. */
-	find_oplock_types(fsp,
-			  oplock_request,
-			  lck,
-			  &batch_entry,
-			  &exclusive_entry,
-			  &got_level2_oplock,
-			  &got_a_none_oplock);
+	if (!validate_oplock_types(fsp, oplock_request, lck,
+				   &got_level2_oplock, &got_a_none_oplock)) {
+		smb_panic("validate_oplock_types failed");
+	}
 
 	/* First pass - send break only on batch oplocks. */
 	if ((req != NULL) &&
-	    delay_for_oplock(fsp, req->mid, oplock_request,
-			     batch_entry)) {
+	    delay_for_oplock(fsp, req->mid, oplock_request, lck,
+			     BATCH_OPLOCK)) {
 		schedule_defer_open(lck, request_time, req);
 		TALLOC_FREE(lck);
 		fd_close(fsp);
@@ -2407,8 +2432,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		/* Second pass - send break for both batch or
 		 * exclusive oplocks. */
 		if ((req != NULL) &&
-		    delay_for_oplock(fsp, req->mid, oplock_request,
-				     exclusive_entry)) {
+		    delay_for_oplock(fsp, req->mid, oplock_request, lck,
+				     BATCH_OPLOCK|EXCLUSIVE_OPLOCK)) {
 			schedule_defer_open(lck, request_time, req);
 			TALLOC_FREE(lck);
 			fd_close(fsp);
-- 
1.8.1.2


From 10a15eb67b768ba3048de052d0c1bdc21051a0ba Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 09:02:12 +0000
Subject: [PATCH 22/25] smbd: Decouple grant_fsp_oplock_type from oplock
 validation

This makes grant_fsp_oplock_type independent from the values computed
in validate_oplock_types. It *might* make oplock calculation a bit
slower for heavily shared files, as we are walking the share mode array
twice. But we are doing so much stuff in open that I doubt the difference
is measurable. It clears up the code for me however, and I think that's
worth it.

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

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 5bed852..eb46ac7 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1214,9 +1214,7 @@ static NTSTATUS send_break_message(files_struct *fsp,
 
 static bool validate_oplock_types(files_struct *fsp,
 				  int oplock_request,
-				  struct share_mode_lock *lck,
-				  bool *p_level2,
-				  bool *p_no_oplock)
+				  struct share_mode_lock *lck)
 {
 	struct share_mode_data *d = lck->data;
 	bool batch = false;
@@ -1230,8 +1228,6 @@ static bool validate_oplock_types(files_struct *fsp,
 		delay_for_exclusive_oplocks().
 	 */
 	if ((oplock_request & INTERNAL_OPEN_ONLY) || is_stat_open(fsp->access_mask)) {
-		*p_level2 = false;
-		*p_no_oplock = false;
 		return true;
 	}
 
@@ -1315,8 +1311,6 @@ static bool validate_oplock_types(files_struct *fsp,
 		return false;
 	}
 
-	*p_level2 = level2;
-	*p_no_oplock = no_oplock;
 	return true;
 }
 
@@ -1369,12 +1363,13 @@ static bool file_has_brlocks(files_struct *fsp)
 }
 
 static void grant_fsp_oplock_type(files_struct *fsp,
-				int oplock_request,
-				bool got_level2_oplock,
-				bool got_a_none_oplock)
+				  struct share_mode_lock *lck,
+				  int oplock_request)
 {
 	bool allow_level2 = (global_client_caps & CAP_LEVEL_II_OPLOCKS) &&
 		            lp_level2_oplocks(SNUM(fsp->conn));
+	bool got_level2_oplock, got_a_none_oplock;
+	uint32_t i;
 
 	/* Start by granting what the client asked for,
 	   but ensure no SAMBA_PRIVATE bits can be set. */
@@ -1401,6 +1396,20 @@ static void grant_fsp_oplock_type(files_struct *fsp,
 		return;
 	}
 
+	got_level2_oplock = false;
+	got_a_none_oplock = false;
+
+	for (i=0; i<lck->data->num_share_modes; i++) {
+		int op_type = lck->data->share_modes[i].op_type;
+
+		if (LEVEL_II_OPLOCK_TYPE(op_type)) {
+			got_level2_oplock = true;
+		}
+		if (op_type == NO_OPLOCK) {
+			got_a_none_oplock = true;
+		}
+	}
+
 	/*
 	 * Match what was requested (fsp->oplock_type) with
  	 * what was found in the existing share modes.
@@ -1975,8 +1984,6 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	NTSTATUS status;
 	char *parent_dir;
 	SMB_STRUCT_STAT saved_stat = smb_fname->st;
-	bool got_level2_oplock = false;
-	bool got_a_none_oplock = false;
 	struct timespec old_write_time;
 	struct file_id id;
 
@@ -2317,8 +2324,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 			return NT_STATUS_SHARING_VIOLATION;
 		}
 
-		if (!validate_oplock_types(fsp, 0, lck, &got_level2_oplock,
-					   &got_a_none_oplock)) {
+		if (!validate_oplock_types(fsp, 0, lck)) {
 			smb_panic("validate_oplock_types failed");
 		}
 
@@ -2407,8 +2413,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	}
 
 	/* Get the types we need to examine. */
-	if (!validate_oplock_types(fsp, oplock_request, lck,
-				   &got_level2_oplock, &got_a_none_oplock)) {
+	if (!validate_oplock_types(fsp, oplock_request, lck)) {
 		smb_panic("validate_oplock_types failed");
 	}
 
@@ -2575,10 +2580,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		return status;
 	}
 
-	grant_fsp_oplock_type(fsp,
-			      oplock_request,
-			      got_level2_oplock,
-			      got_a_none_oplock);
+	grant_fsp_oplock_type(fsp, lck, oplock_request);
 
 	/*
 	 * We have the share entry *locked*.....
-- 
1.8.1.2


From d6dd487fa49a6583d9acd6a7f2df8cb13466fe84 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:27:49 +0000
Subject: [PATCH 23/25] smbd: Unify parameters to set_oplock_type

Some lines above we set fsp->oplock_type = e->op_type. I don't see
how this might have changed. This change will unify both callers of
set_file_oplock. In the next step the second parameter to set_file_oplock
will go.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/durable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 9b05d48..15d7005 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -864,7 +864,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 	}
 
-	status = set_file_oplock(fsp, e->op_type);
+	status = set_file_oplock(fsp, fsp->oplock_type);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("vfs_default_durable_reconnect failed to set oplock "
 			  "after opening file: %s\n", nt_errstr(status)));
-- 
1.8.1.2


From abb80a787f1d601e5a357270b3927ccf7e51c1c5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:31:27 +0000
Subject: [PATCH 24/25] smbd: Fix confusing comments

The brlock-check is done in grant_fsp_oplock_type

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/open.c   | 3 +--
 source3/smbd/oplock.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index eb46ac7..9d1edda 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2669,8 +2669,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	status = set_file_oplock(fsp, fsp->oplock_type);
 	if (!NT_STATUS_IS_OK(status)) {
 		/*
-		 * Could not get the kernel oplock or there are byte-range
-		 * locks on the file.
+		 * Could not get the kernel oplock
 		 */
 		fsp->oplock_type = NO_OPLOCK;
 	}
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 8253fd9..ad054f1 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -47,8 +47,7 @@ void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp)
 
 /****************************************************************************
  Attempt to set an oplock on a file. Succeeds if kernel oplocks are
- disabled (just sets flags) and no byte-range locks in the file. Returns True
- if oplock set.
+ disabled (just sets flags).
 ****************************************************************************/
 
 NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
-- 
1.8.1.2


From 337c2931f1daf23446eec02856ed9a505018209d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Sep 2013 13:57:11 +0000
Subject: [PATCH 25/25] smbd: Remove separate oplock_type parameter from
 set_file_oplock

This was pretty confusing, both callers used fsp->oplock_type here anyway

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/durable.c | 2 +-
 source3/smbd/open.c    | 2 +-
 source3/smbd/oplock.c  | 7 +++----
 source3/smbd/proto.h   | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 15d7005..c5281a8 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -864,7 +864,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 	}
 
-	status = set_file_oplock(fsp, fsp->oplock_type);
+	status = set_file_oplock(fsp);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("vfs_default_durable_reconnect failed to set oplock "
 			  "after opening file: %s\n", nt_errstr(status)));
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 9d1edda..1a07889 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2666,7 +2666,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	 * file structs.
 	 */
 
-	status = set_file_oplock(fsp, fsp->oplock_type);
+	status = set_file_oplock(fsp);
 	if (!NT_STATUS_IS_OK(status)) {
 		/*
 		 * Could not get the kernel oplock
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index ad054f1..881fd36 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -50,7 +50,7 @@ void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp)
  disabled (just sets flags).
 ****************************************************************************/
 
-NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
+NTSTATUS set_file_oplock(files_struct *fsp)
 {
 	struct smbd_server_connection *sconn = fsp->conn->sconn;
 	struct kernel_oplocks *koplocks = sconn->oplocks.kernel_ops;
@@ -68,14 +68,13 @@ NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type)
 	if ((fsp->oplock_type != NO_OPLOCK) &&
 	    (fsp->oplock_type != FAKE_LEVEL_II_OPLOCK) &&
 	    use_kernel &&
-	    !koplocks->ops->set_oplock(koplocks, fsp, oplock_type))
+	    !koplocks->ops->set_oplock(koplocks, fsp, fsp->oplock_type))
 	{
 		return map_nt_error_from_unix(errno);
 	}
 
-	fsp->oplock_type = oplock_type;
 	fsp->sent_oplock_break = NO_BREAK_SENT;
-	if (oplock_type == LEVEL_II_OPLOCK) {
+	if (fsp->oplock_type == LEVEL_II_OPLOCK) {
 		sconn->oplocks.level_II_open++;
 	} else if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
 		sconn->oplocks.exclusive_open++;
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 8b6987e..5dac5ef 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -659,7 +659,7 @@ NTSTATUS get_relative_fid_filename(connection_struct *conn,
 /* The following definitions come from smbd/oplock.c  */
 
 void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp);
-NTSTATUS set_file_oplock(files_struct *fsp, int oplock_type);
+NTSTATUS set_file_oplock(files_struct *fsp);
 void release_file_oplock(files_struct *fsp);
 bool remove_oplock(files_struct *fsp);
 bool downgrade_oplock(files_struct *fsp);
-- 
1.8.1.2



More information about the samba-technical mailing list