Attachment 9208 Details for Bug 10138 – git-am fix for 4.1.0

[patch] git-am fix for 4.1.0

bug-10138-4.1.patchset (text/plain), 14.98 KB, created by Jeremy Allison on 2013-09-12 19:19:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2013-09-12 19:19:34 UTC

Size: 14.98 KB

patch

obsolete

>From 4afbb4a94f6b259544728723291bece59ed59231 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 22 Aug 2013 08:49:07 +0000
>Subject: [PATCH 1/5] smbd: Simplify find_oplock_types
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>(cherry picked from commit 94b320527eee0c7ba1d3818816e7d59cb863bf3f)
>---
> 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 dcf6bb5..10b04e7 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -1251,19 +1251,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"));
>@@ -1272,10 +1273,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;
>@@ -1284,10 +1285,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 "
>@@ -1299,7 +1300,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.4
>
>
>From a3d150c6eb9fc6419579f3934e9c098447d3d7fd Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 1 Sep 2013 11:07:19 +0200
>Subject: [PATCH 2/5] smbd: Don't store in-memory only flags in locking.tdb
>
>Hey, pidl knows the [skip] attribute ... :-)
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>(cherry picked from commit 696bc569b17f024f840774e3d59761229836a310)
>---
> 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.4
>
>
>From 64d66d3f5363942e6ac0fdabd062dcbc861eda4e Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Fri, 30 Aug 2013 12:27:36 +0000
>Subject: [PATCH 3/5] smbd: Rename parameter "i" to "idx"
>
>We'll need "i" in a later checkin ... :-)
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>(cherry picked from commit 5006db98aaf1efe119f1da8be091587a9bc2b952)
>---
> 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.4
>
>
>From da5b75e8efc878191f3867a150937fcf91052dfe Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Fri, 30 Aug 2013 12:49:43 +0000
>Subject: [PATCH 4/5] 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@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>(cherry picked from commit 7d91ffc6fdc3b371564e14f09822a96264ea372a)
>---
> 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.4
>
>
>From 5cb1dfe73dfa06dd1fdcf8e58370085098f9c838 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Sun, 1 Sep 2013 18:54:59 +0200
>Subject: [PATCH 5/5] torture3: Trigger a nasty cleanup bug in smbd
>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>
>Autobuild-User(master): Michael Adam <obnox@samba.org>
>Autobuild-Date(master): Tue Sep  3 19:13:14 CEST 2013 on sn-devel-104
>
>(cherry picked from commit ade8477f98fcffcc6e3c5ea31618b49d0c1bba95)
>---
> 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 3fc6684..e5ae63e 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 4f4c9e2..c9fc2c5 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 c6c5322..ee51a4d 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -9509,6 +9509,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.4
>

Actions: View

Attachments on bug 10138: 9208 | 9209 | 9247