Attachment 14217 Details for Bug 13452 – a possible patch

[patch] a possible patch

0001-ldb-Save-a-copy-of-the-index-result-before-calling-t.patch.txt (text/plain), 3.79 KB, created by Andrew Bartlett on 2018-05-28 01:19:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2018-05-28 01:19:35 UTC

Size: 3.79 KB

patch

obsolete

>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
>

Actions: View

Attachments on bug 13452: 14217