[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Mon Aug 31 13:35:03 UTC 2020


The branch, master has been updated
       via  b02f1d676f6 s3:share_mode_lock: remove unused reproducer for bug #14428
       via  b5c0874fd5d s3:share_mode_lock: make sure share_mode_cleanup_disconnected() removes the record
       via  1aa1ac97082 s3:share_mode_lock: add missing 'goto done' in share_mode_cleanup_disconnected()
       via  4d740ac2084 s3:share_mode_lock: consistently debug share_mode_entry records
       via  deb2f782c95 s3:share_mode_lock: let share_mode_forall_entries/share_entry_forall evaluate e.stale first
       via  444f2bedf72 s3:share_mode_lock: reproduce problem with stale disconnected share mode entries
       via  560fe7b38f0 s3:selftest: also run durable_v2_reconnect_delay_msec in samba3.blackbox.durable_v2_delay
      from  232054c09b1 lib/util: remove extra safe_string.h file

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit b02f1d676f6e62a0a4b33b9b08f8f51a68b561ca
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 16:28:41 2020 +0200

    s3:share_mode_lock: remove unused reproducer for bug #14428
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Mon Aug 31 13:34:17 UTC 2020 on sn-devel-184

commit b5c0874fd5d31e252cf9ac8b84bde5c536b1e8ef
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 16:28:41 2020 +0200

    s3:share_mode_lock: make sure share_mode_cleanup_disconnected() removes the record
    
    This fixes one possible trigger for "PANIC: assert failed in get_lease_type()"
    https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    This is no longer enough to remove the record:
    
       d->have_share_modes = false;
       d->modified = true;
    
    Note that we can remove it completely from
    share_mode_cleanup_disconnected() as
    share_mode_forall_entries() already sets it
    when there are no entries left.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 1aa1ac97082f81f6dc62f345823d2dd345e0afd7
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 15:56:35 2020 +0200

    s3:share_mode_lock: add missing 'goto done' in share_mode_cleanup_disconnected()
    
    When cleanup_disconnected_lease() fails we should stop,
    at least we do that if brl_cleanup_disconnected() fails.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 4d740ac2084a68c6d4836cd83ea5d5f1ee9d37a2
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 15:56:35 2020 +0200

    s3:share_mode_lock: consistently debug share_mode_entry records
    
    share_mode_entry_do(), share_mode_forall_entries() and
    share_entry_forall() print the record before the callback is called
    and when it was modified or deleted.
    
    This makes it much easier to debug problems.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit deb2f782c95a5e59a0a5da4272239c2d31bc2b6d
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 15:56:35 2020 +0200

    s3:share_mode_lock: let share_mode_forall_entries/share_entry_forall evaluate e.stale first
    
    It's not really clear why e.stale would be ignored if *modified is set
    to true.
    
    This matches the behavior of share_mode_entry_do()
    
    This also makes sure we see the removed entry in level 10 logs again.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 444f2bedf723b89bb9f493c47812bff2154c4113
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 14:37:59 2020 +0200

    s3:share_mode_lock: reproduce problem with stale disconnected share mode entries
    
    This reproduces the origin of "PANIC: assert failed in get_lease_type()"
    (https://bugzilla.samba.org/show_bug.cgi?id=14428).
    
    share_mode_cleanup_disconnected() removes disconnected entries from
    leases.tdb and brlock.tdb but not from locking.tdb.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 560fe7b38f0c8d53079fabf3f984b11748270035
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Aug 28 15:33:43 2020 +0200

    s3:selftest: also run durable_v2_reconnect_delay_msec in samba3.blackbox.durable_v2_delay
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14428
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

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

Summary of changes:
 source3/locking/share_mode_lock.c                  | 104 +++++++++++++++++----
 .../script/tests/test_durable_handle_reconnect.sh  |  12 +++
 2 files changed, 100 insertions(+), 16 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index ba0bc2b1e7b..1c4d3a42221 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -1648,6 +1648,42 @@ static bool share_mode_find_connected_fn(
 	return false;
 }
 
+static bool cleanup_disconnected_share_mode_entry_fn(
+	struct share_mode_entry *e,
+	bool *modified,
+	void *private_data)
+{
+	struct cleanup_disconnected_state *state = private_data;
+	struct share_mode_data *d = state->lck->data;
+
+	bool disconnected;
+
+	disconnected = server_id_is_disconnected(&e->pid);
+	if (!disconnected) {
+		struct file_id_buf tmp1;
+		struct server_id_buf tmp2;
+		DBG_ERR("file (file-id='%s', servicepath='%s', "
+			"base_name='%s%s%s') "
+			"is used by server %s ==> internal error\n",
+			file_id_str_buf(d->id, &tmp1),
+			d->servicepath,
+			d->base_name,
+			(d->stream_name == NULL)
+			? "" : "', stream_name='",
+			(d->stream_name == NULL)
+			? "" : d->stream_name,
+			server_id_str_buf(e->pid, &tmp2));
+		smb_panic(__location__);
+	}
+
+	/*
+	 * Setting e->stale = true is
+	 * the indication to delete the entry.
+	 */
+	e->stale = true;
+	return false;
+}
+
 bool share_mode_cleanup_disconnected(struct file_id fid,
 				     uint64_t open_persistent_id)
 {
@@ -1694,6 +1730,7 @@ bool share_mode_cleanup_disconnected(struct file_id fid,
 			  (data->stream_name == NULL)
 			  ? "" : data->stream_name,
 			  open_persistent_id);
+		goto done;
 	}
 
 	ok = brl_cleanup_disconnected(fid, open_persistent_id);
@@ -1727,8 +1764,24 @@ bool share_mode_cleanup_disconnected(struct file_id fid,
 		  ? "" : data->stream_name,
 		  open_persistent_id);
 
-	data->have_share_modes = false;
-	data->modified = true;
+	ok = share_mode_forall_entries(
+		state.lck, cleanup_disconnected_share_mode_entry_fn, &state);
+	if (!ok) {
+		DBG_DEBUG("failed to clean up %zu entries associated "
+			  "with file (file-id='%s', servicepath='%s', "
+			  "base_name='%s%s%s') and open_persistent_id %"PRIu64" "
+			  "==> do not cleanup\n",
+			  state.num_disconnected,
+			  file_id_str_buf(fid, &idbuf),
+			  data->servicepath,
+			  data->base_name,
+			  (data->stream_name == NULL)
+			  ? "" : "', stream_name='",
+			  (data->stream_name == NULL)
+			  ? "" : data->stream_name,
+			  open_persistent_id);
+		goto done;
+	}
 
 	ret = true;
 done:
@@ -2008,6 +2061,23 @@ static bool share_mode_for_one_entry(
 		  (int)modified,
 		  (int)e.stale);
 
+	if (e.stale) {
+		if (DEBUGLEVEL>=10) {
+			DBG_DEBUG("share_mode_entry:\n");
+			NDR_PRINT_DEBUG(share_mode_entry, &e);
+		}
+
+		if (*i < *num_share_modes) {
+			memmove(blob.data,
+				blob.data + SHARE_MODE_ENTRY_SIZE,
+				(*num_share_modes - *i - 1) *
+				SHARE_MODE_ENTRY_SIZE);
+		}
+		*num_share_modes -= 1;
+		*writeback = true;
+		return stop;
+	}
+
 	if (modified) {
 		if (DEBUGLEVEL>=10) {
 			DBG_DEBUG("share_mode_entry:\n");
@@ -2038,18 +2108,6 @@ static bool share_mode_for_one_entry(
 		return stop;
 	}
 
-	if (e.stale) {
-		if (*i < *num_share_modes) {
-			memmove(blob.data,
-				blob.data + SHARE_MODE_ENTRY_SIZE,
-				(*num_share_modes - *i - 1) *
-				SHARE_MODE_ENTRY_SIZE);
-		}
-		*num_share_modes -= 1;
-		*writeback = true;
-		return stop;
-	}
-
 	if (stop) {
 		return true;
 	}
@@ -2226,8 +2284,18 @@ static bool share_mode_entry_do(
 		goto done;
 	}
 
+	if (DEBUGLEVEL>=10) {
+		DBG_DEBUG("entry[%zu]:\n", idx);
+		NDR_PRINT_DEBUG(share_mode_entry, &e);
+	}
+
 	fn(&e, ltdb->num_share_entries, &modified, private_data);
 
+	DBG_DEBUG("entry[%zu]: modified=%d, e.stale=%d\n",
+		  idx,
+		  (int)modified,
+		  (int)e.stale);
+
 	if (!e.stale && !modified) {
 		ret = true;
 		goto done;
@@ -2238,9 +2306,8 @@ static bool share_mode_entry_do(
 
 	if (e.stale) {
 		/*
-		 * Move the reset down one entry
+		 * Move the rest down one entry
 		 */
-
 		size_t behind = ltdb->num_share_entries - idx - 1;
 		if (behind != 0) {
 			memmove(e_ptr,
@@ -2248,6 +2315,11 @@ static bool share_mode_entry_do(
 				behind * SHARE_MODE_ENTRY_SIZE);
 		}
 		ltdb->num_share_entries -= 1;
+
+		if (DEBUGLEVEL>=10) {
+			DBG_DEBUG("share_mode_entry:\n");
+			NDR_PRINT_DEBUG(share_mode_entry, &e);
+		}
 	} else {
 		struct share_mode_entry_buf buf;
 		bool ok;
diff --git a/source3/script/tests/test_durable_handle_reconnect.sh b/source3/script/tests/test_durable_handle_reconnect.sh
index 77d82c1b403..2f6b308eebe 100755
--- a/source3/script/tests/test_durable_handle_reconnect.sh
+++ b/source3/script/tests/test_durable_handle_reconnect.sh
@@ -17,6 +17,18 @@ testit "durable_v2_delay.durable_v2_reconnect_delay" $VALGRIND \
        smb2.durable-v2-delay.durable_v2_reconnect_delay ||
 	failed=$(expr $failed + 1)
 
+SMBD_LOG_FILES="$SMBD_TEST_LOG"
+if [ $SMBD_DONT_LOG_STDOUT -eq 1 ]; then
+	_SMBD_LOG_FILE=$(dirname $SMBD_TEST_LOG)/logs/log.smbd
+	SMBD_LOG_FILES="$SMBD_LOG_FILES $_SMBD_LOG_FILE"
+fi
+
+testit "durable_v2_delay.durable_v2_reconnect_delay_msec" $VALGRIND \
+       $BINDIR/smbtorture //$SERVER_IP/durable \
+       -U$USERNAME%$PASSWORD \
+       smb2.durable-v2-delay.durable_v2_reconnect_delay_msec ||
+	failed=$(expr $failed + 1)
+
 rm $delay_inject_conf
 
 testok $0 $failed


-- 
Samba Shared Repository



More information about the samba-cvs mailing list