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