From c2234c300c678f3e7a1d17bef1fb061c03e89bd6 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Mon, 28 May 2018 13:01:18 +1200
Subject: [PATCH] ldb: Save a copy of the index result before calling the
 callbacks.

Otherwise Samba modules like subtree_rename can fail as they modify the
index during the callback.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13452

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
---
 lib/ldb/ldb_tdb/ldb_index.c | 80 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 18 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 627424ae8f4..e341cb5426f 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1717,26 +1717,63 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 			     uint32_t *match_count,
 			     enum key_truncation scope_one_truncation)
 {
-	struct ldb_context *ldb;
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
 	struct ldb_message *msg;
 	struct ldb_message *filtered_msg;
 	unsigned int i;
+	unsigned int num_keys = 0;
 	uint8_t previous_guid_key[LTDB_GUID_KEY_SIZE] = {};
 
-	ldb = ldb_module_get_ctx(ac->module);
+	struct guid_tdb_key {
+		uint8_t guid_key[LTDB_GUID_KEY_SIZE];
+	};
+
+	/*
+	 * We have to allocate the key list (rather than just walk the
+	 * caller supplied list) as the callback could change the list
+	 * (by modifying an indexed attribute hosted in the in-memory
+	 * index cache!)
+	 */
+	TDB_DATA *keys = talloc_array(ac, TDB_DATA, dn_list->count);
+
+	/*
+	 * We speculate that the keys will be GUID based and so
+	 * pre-fill in enough space for a GUID (avoiding a pile of
+	 * small allocations)
+	 */
+	struct guid_tdb_key *key_values;
+
+	if (keys == NULL) {
+		return ldb_module_oom(ac->module);
+	}
+
+	if (ltdb->cache->GUID_index_attribute != NULL) {
+		key_values = talloc_array(ac,
+					  struct guid_tdb_key,
+					  dn_list->count);
+
+		for (i = 0; i < dn_list->count; i++) {
+			keys[i].dptr = key_values[i].guid_key;
+			keys[i].dsize = sizeof(key_values[i].guid_key);
+		}
+		if (key_values == NULL) {
+			return ldb_module_oom(ac->module);
+		}
+	} else {
+		for (i = 0; i < dn_list->count; i++) {
+			keys[i].dptr = NULL;
+			keys[i].dsize = 0;
+		}
+	}
 
 	for (i = 0; i < dn_list->count; i++) {
-		uint8_t guid_key[LTDB_GUID_KEY_SIZE];
-		TDB_DATA tdb_key = {
-			.dptr = guid_key,
-			.dsize = sizeof(guid_key)
-		};
 		int ret;
-		bool matched;
 
-		ret = ltdb_idx_to_key(ac->module, ltdb,
-				      ac, &dn_list->dn[i],
-				      &tdb_key);
+		ret = ltdb_idx_to_key(ac->module,
+				      ltdb,
+				      keys,
+				      &dn_list->dn[i],
+				      &keys[num_keys]);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
@@ -1753,28 +1790,35 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 			 * LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK
 			 */
 
-			if (memcmp(previous_guid_key, tdb_key.dptr,
+			if (memcmp(previous_guid_key,
+				   keys[num_keys].dptr,
 				   sizeof(previous_guid_key)) == 0) {
 				continue;
 			}
 
-			memcpy(previous_guid_key, tdb_key.dptr,
+			memcpy(previous_guid_key,
+			       keys[num_keys].dptr,
 			       sizeof(previous_guid_key));
 		}
+		num_keys++;
+	}
+
 
+	/*
+	 * Now that the list is a safe copy, send the callbacks
+	 */
+	for (i = 0; i < num_keys; i++) {
+		int ret;
+		bool matched;
 		msg = ldb_msg_new(ac);
 		if (!msg) {
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
 
-
 		ret = ltdb_search_key(ac->module, ltdb,
-				      tdb_key, msg,
+				      keys[i], msg,
 				      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
 				      LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
-		if (tdb_key.dptr != guid_key) {
-			TALLOC_FREE(tdb_key.dptr);
-		}
 		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
 			/* the record has disappeared? yes, this can happen */
 			talloc_free(msg);
-- 
2.14.3