ldb: Add mem_ctx argument to ldb_kv_index_key()

This avoids using "ldb" as the memory context in most cases, and may avoid
a long-term memory leak if future changes cause dn_key not to be freed.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>

1 files changed, 43 insertions, 15 deletions

diff --git a/lib/ldb/ldb_key_value/ldb_kv_index.c b/lib/ldb/ldb_key_value/ldb_kv_index.c
index 2935b95c4dd..5a24b074e1c 100644
--- a/lib/ldb/ldb_key_value/ldb_kv_index.c
+++ b/lib/ldb/ldb_key_value/ldb_kv_index.c
@@ -975,6 +975,7 @@ int ldb_kv_index_transaction_cancel(struct ldb_module *module)
   the caller is responsible for freeing
 */
 static struct ldb_dn *ldb_kv_index_key(struct ldb_context *ldb,
+				       TALLOC_CTX *mem_ctx,
 				       struct ldb_kv_private *ldb_kv,
 				       const char *attr,
 				       const struct ldb_val *value,
@@ -1110,7 +1111,7 @@ static struct ldb_dn *ldb_kv_index_key(struct ldb_context *ldb,
 
 	if (should_b64_encode) {
 		size_t vstr_len = 0;
-		char *vstr = ldb_base64_encode(ldb, (char *)v.data, v.length);
+		char *vstr = ldb_base64_encode(mem_ctx, (char *)v.data, v.length);
 		if (!vstr) {
 			talloc_free(attr_folded);
 			return NULL;
@@ -1131,7 +1132,7 @@ static struct ldb_dn *ldb_kv_index_key(struct ldb_context *ldb,
 			* Note: the double hash "##" is not a typo and
 			* indicates that the following value is base64 encoded
 			*/
-			ret = ldb_dn_new_fmt(ldb, ldb, "%s#%s##%.*s",
+			ret = ldb_dn_new_fmt(mem_ctx, ldb, "%s#%s##%.*s",
 					     LDB_KV_INDEX, attr_for_dn,
 					     frmt_len, vstr);
 		} else {
@@ -1141,7 +1142,7 @@ static struct ldb_dn *ldb_kv_index_key(struct ldb_context *ldb,
 			 * Note: the double colon "::" is not a typo and
 			 * indicates that the following value is base64 encoded
 			 */
-			ret = ldb_dn_new_fmt(ldb, ldb, "%s:%s::%.*s",
+			ret = ldb_dn_new_fmt(mem_ctx, ldb, "%s:%s::%.*s",
 					     LDB_KV_INDEX, attr_for_dn,
 					     frmt_len, vstr);
 		}
@@ -1163,13 +1164,13 @@ static struct ldb_dn *ldb_kv_index_key(struct ldb_context *ldb,
 			 * Truncated keys are placed in a separate key space
 			 * from the non truncated keys
 			 */
-			ret = ldb_dn_new_fmt(ldb, ldb, "%s#%s#%.*s",
+			ret = ldb_dn_new_fmt(mem_ctx, ldb, "%s#%s#%.*s",
 					     LDB_KV_INDEX, attr_for_dn,
 					     frmt_len, (char *)v.data);
 		} else {
 			frmt_len = v.length;
 			*truncation = KEY_NOT_TRUNCATED;
-			ret = ldb_dn_new_fmt(ldb, ldb, "%s:%s:%.*s",
+			ret = ldb_dn_new_fmt(mem_ctx, ldb, "%s:%s:%.*s",
 					     LDB_KV_INDEX, attr_for_dn,
 					     frmt_len, (char *)v.data);
 		}
@@ -1269,9 +1270,15 @@ static int ldb_kv_index_dn_simple(struct ldb_module *module,
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	/* the attribute is indexed. Pull the list of DNs that match the
-	   search criterion */
+	/*
+	 * the attribute is indexed. Pull the list of DNs that match the
+	 * search criterion
+	 *
+	 * list is used as a memory context as it has a shorter life
+	 * than 'ldb'.  Regardless we talloc_free() 'dn' below.
+	 */
 	dn = ldb_kv_index_key(ldb,
+			      list,
 			      ldb_kv,
 			      tree->u.equality.attr,
 			      &tree->u.equality.value,
@@ -1957,7 +1964,7 @@ static int ldb_kv_index_dn_ordered(struct ldb_module *module,
 		return ldb_module_oom(module);
 	}
 
-	key_dn = ldb_kv_index_key(ldb, ldb_kv, tree->u.comparison.attr,
+	key_dn = ldb_kv_index_key(ldb, tmp_ctx, ldb_kv, tree->u.comparison.attr,
 				  &tree->u.comparison.value,
 				  NULL, &truncation);
 	if (!key_dn) {
@@ -1978,7 +1985,8 @@ static int ldb_kv_index_dn_ordered(struct ldb_module *module,
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	key_dn = ldb_kv_index_key(ldb, ldb_kv, tree->u.comparison.attr,
+	key_dn = ldb_kv_index_key(ldb, tmp_ctx,
+				  ldb_kv, tree->u.comparison.attr,
 				  NULL, NULL, &truncation);
 	if (!key_dn) {
 		TALLOC_FREE(tmp_ctx);
@@ -2098,7 +2106,13 @@ static int ldb_kv_index_dn_attr(struct ldb_module *module,
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 	val.length = strlen((char *)val.data);
-	key = ldb_kv_index_key(ldb, ldb_kv, attr, &val, NULL, truncation);
+
+	/*
+	 * We use list as a TALLOC_CTX to provide a shorter-lived
+	 * memory context than ldb, even as the result is freed with
+	 * the talloc_free(key) below.
+	 */
+	key = ldb_kv_index_key(ldb, list, ldb_kv, attr, &val, NULL, truncation);
 	if (!key) {
 		ldb_oom(ldb);
 		return LDB_ERR_OPERATIONS_ERROR;
@@ -2670,8 +2684,13 @@ static int ldb_kv_index_add1(struct ldb_module *module,
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	dn_key = ldb_kv_index_key(
-	    ldb, ldb_kv, el->name, &el->values[v_idx], &a, &truncation);
+	dn_key = ldb_kv_index_key(ldb,
+				  list,
+				  ldb_kv,
+				  el->name,
+				  &el->values[v_idx],
+				  &a,
+				  &truncation);
 	if (!dn_key) {
 		talloc_free(list);
 		return LDB_ERR_OPERATIONS_ERROR;
@@ -2695,7 +2714,6 @@ static int ldb_kv_index_add1(struct ldb_module *module,
 		talloc_free(list);
 		return LDB_ERR_CONSTRAINT_VIOLATION;
 	}
-	talloc_steal(list, dn_key);
 
 	ret = ldb_kv_dn_list_load(module, ldb_kv, dn_key, list,
 				  DN_LIST_MUTABLE);
@@ -3195,8 +3213,18 @@ int ldb_kv_index_del_value(struct ldb_module *module,
 		return LDB_SUCCESS;
 	}
 
-	dn_key = ldb_kv_index_key(
-	    ldb, ldb_kv, el->name, &el->values[v_idx], NULL, &truncation);
+	/*
+	 * ldb is being used as the memory context to ldb_kv_index_key
+	 * as dn_key itself is also used as the TALLOC_CTX for the
+	 * rest of this function.
+	 */
+	dn_key = ldb_kv_index_key(ldb,
+				  ldb,
+				  ldb_kv,
+				  el->name,
+				  &el->values[v_idx],
+				  NULL,
+				  &truncation);
 	/*
 	 * We ignore key truncation in ltdb_index_add1() so
 	 * match that by ignoring it here as well