[SCM] Samba Shared Repository - branch v4-0-test updated

Karolin Seeger kseeger at samba.org
Tue Oct 1 03:29:01 MDT 2013


The branch, v4-0-test has been updated
       via  825aadb WHATSNEW: Add latest changes since 4.0.9.
       via  8303c26 smbd: Fix crash bug in notify_deferred_opens
       via  76952d4 torture3: Trigger a nasty cleanup bug in smbd
       via  73166e5 smbd: Fix flawed share_mode_stale_pid API
       via  65c8909 smbd: Rename parameter "i" to "idx"
       via  e3e0f59 smbd: Don't store in-memory only flags in locking.tdb
       via  a321024 smbd: Simplify find_oplock_types
      from  c02af3e WHATSNEW: Add hint on the new "acl allow execute always" parameter.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v4-0-test


- Log -----------------------------------------------------------------
commit 825aadbdc46a4a10ee923082f69620112d109070
Author: Karolin Seeger <kseeger at samba.org>
Date:   Tue Oct 1 09:36:11 2013 +0200

    WHATSNEW: Add latest changes since 4.0.9.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>
    
    Autobuild-User(v4-0-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-0-test): Tue Oct  1 11:28:04 CEST 2013 on sn-devel-104

commit 8303c26ae93e487d823ab80251b1e9f6ff85d9ea
Author: Volker Lendecke <vl at samba.org>
Date:   Mon Sep 30 12:52:49 2013 +0000

    smbd: Fix crash bug in notify_deferred_opens
    
    The "deferred" array only holds enough entries for non-stale pids. We
    should skip those as well when filling that array.
    
    This bug came in with 19b6671. No issue in master and 4.1, we don't have
    deferred entries anymore there.
    
    Part of a fix for bug #10138 - smbd doesn't always clean up share modes after
    hard crash.

commit 76952d4ac5fdc96109e50e2aa10e7a0f1326de1e
Author: Volker Lendecke <vl at samba.org>
Date:   Sun Sep 1 18:54:59 2013 +0200

    torture3: Trigger a nasty cleanup bug in smbd
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>
    
    Autobuild-User(master): Michael Adam <obnox at samba.org>
    Autobuild-Date(master): Tue Sep  3 19:13:14 CEST 2013 on sn-devel-104
    
    (cherry picked from commit ade8477f98fcffcc6e3c5ea31618b49d0c1bba95)
    
    The latest 5 patches address bug #10138 - smbd doesn't always clean up share
    modes after hard crash.

commit 73166e5fde3e587ba9e0e204424973ca0869cf54
Author: Volker Lendecke <vl at samba.org>
Date:   Fri Aug 30 12:49:43 2013 +0000

    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>
    Reviewed-by: Michael Adam <obnox at samba.org>
    (cherry picked from commit 7d91ffc6fdc3b371564e14f09822a96264ea372a)

commit 65c89097475205a21892c35c62fb8cf977390ef7
Author: Volker Lendecke <vl at samba.org>
Date:   Fri Aug 30 12:27:36 2013 +0000

    smbd: Rename parameter "i" to "idx"
    
    We'll need "i" in a later checkin ... :-)
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>
    (cherry picked from commit 5006db98aaf1efe119f1da8be091587a9bc2b952)
    
    Conflicts:
    	source3/locking/proto.h

commit e3e0f591361a51a911df5d60590ae6d462567c89
Author: Volker Lendecke <vl at samba.org>
Date:   Sun Sep 1 11:07:19 2013 +0200

    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>
    Reviewed-by: Michael Adam <obnox at samba.org>
    (cherry picked from commit 696bc569b17f024f840774e3d59761229836a310)

commit a321024a5beb32b7ce4cd4bf0ea9195911b83ad9
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Aug 22 08:49:07 2013 +0000

    smbd: Simplify find_oplock_types
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>
    (cherry picked from commit 94b320527eee0c7ba1d3818816e7d59cb863bf3f)

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

Summary of changes:
 WHATSNEW.txt                      |    2 +
 source3/librpc/idl/open_files.idl |   10 ++++-
 source3/locking/locking.c         |   47 +++++++++++++++++--------
 source3/locking/proto.h           |    2 +-
 source3/locking/share_mode_lock.c |   24 +++++++++++++
 source3/selftest/tests.py         |    1 +
 source3/smbd/close.c              |   10 ++++--
 source3/smbd/open.c               |   19 +++++-----
 source3/torture/proto.h           |    1 +
 source3/torture/test_cleanup.c    |   70 +++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c         |    1 +
 11 files changed, 157 insertions(+), 30 deletions(-)


Changeset truncated at 500 lines:

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 932c90e..b6d0c72 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -9,6 +9,7 @@ This is is the latest stable release of Samba 4.0.
 Major enhancements in Samba 4.0.10 include:
 
 o  NetBIOS related samba process consumes 100% CPU (bug #10158).
+o  smbd: Clean up share modes after hard crash (bug #10138).
 o  Fix POSIX ACL mapping when setting DENY ACE's from Windows (bug #10162).
 
 To ease upgrades from Samba 3.6 and older, a new parameter called "acl allow
@@ -70,6 +71,7 @@ o   Volker Lendecke <vl at samba.org>
       GetInfo requests.
     * BUG 10114: Dropbox (write-only-directory) case isn't handled correctly in
       pathname lookup.
+    * BUG 10138: smbd: Clean up share modes after hard crash.
 
 
 o   Daniel Liberman <danielvl at gmail.com>
diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl
index fa87bc7..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 {
@@ -42,8 +48,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;
 
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index a7fc50c..be2c92d 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);
@@ -635,40 +639,53 @@ bool is_deferred_open_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 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;
 
-	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/proto.h b/source3/locking/proto.h
index bb7255d..47d9b80 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -171,7 +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 is_deferred_open_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);
 void add_deferred_open(struct share_mode_lock *lck, uint64_t mid,
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 266be65..6782f59 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -118,6 +118,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);
@@ -137,6 +138,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;
 
@@ -159,12 +168,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);
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 4453c4d..c293e6d 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/smbd/close.c b/source3/smbd/close.c
index e5b1fb7..4adcc61 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -204,10 +204,14 @@ static void notify_deferred_opens(struct smbd_server_connection *sconn,
 	num_deferred = 0;
 	for (i=0; i<lck->data->num_share_modes; i++) {
 		struct share_mode_entry *e = &lck->data->share_modes[i];
-		if (is_deferred_open_entry(e)) {
-			deferred[num_deferred] = *e;
-			num_deferred += 1;
+		if (!is_deferred_open_entry(e)) {
+			continue;
+		}
+		if (share_mode_stale_pid(lck->data, i)) {
+			continue;
 		}
+		deferred[num_deferred] = *e;
+		num_deferred += 1;
 	}
 
 	/*
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 0eb1ec2..a1b4e43 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1257,19 +1257,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"));
@@ -1278,10 +1279,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;
@@ -1290,10 +1291,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 "
@@ -1305,7 +1306,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 "
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 0c6fc70..dde28ef 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 21dcf9b..107106c 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -9508,6 +9508,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},


-- 
Samba Shared Repository


More information about the samba-cvs mailing list