From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 28 Aug 2020 14:28:41 +0000 (+0200)
Subject: s3:share_mode_lock: make sure share_mode_cleanup_disconnected() removes the record
X-Git-Tag: talloc-2.3.2~676
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b5c0874fd5d31e252cf9ac8b84bde5c536b1e8ef;p=thirdparty%2Fsamba.git

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@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
---

diff --git a/selftest/knownfail.d/bug-14428 b/selftest/knownfail.d/bug-14428
deleted file mode 100644
index e198400ea16..00000000000
--- a/selftest/knownfail.d/bug-14428
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.durable_v2_delay.bug.14428
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 965c5ab1883..6588f09b285 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)
 {
@@ -1728,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;
+	}
 
 	/*
 	 * This is a temporary reproducer for the origin of