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>

2 files changed, 50 insertions, 9 deletions

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 602c30dfa2..5090082ba8 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 0693cf5408..342f9108d6 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);