From: Andrew Bartlett <abartlet@samba.org>
Date: Tue, 24 Aug 2021 21:41:11 +0000 (+1200)
Subject: dsdb: Be careful to avoid use of the expensive talloc_is_parent()
X-Git-Tag: ldb-2.5.0~781
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8affe4a1e625104de4ca024fdc3e9cd96498aff3;p=thirdparty%2Fsamba.git

dsdb: Be careful to avoid use of the expensive talloc_is_parent()

The wrong talloc API was selected while addressing a memory leak.

commit ee2fe56ba0ef6626b634376e8dc2185aa89f8c99
Author: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Date:   Tue Nov 27 11:07:44 2018 +1300

    drepl: memory leak fix

    Fixes a memory leak where schema reference attached to ldb
    instance is lost before it can be freed.

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

    Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>

    Reviewed-by: Andrew Bartlett <abartlet@samba.org>
    Reviewed-by: Garming Sam <garming@catalyst.net.nz>

    Autobuild-User(master): Garming Sam <garming@samba.org>
    Autobuild-Date(master): Wed Jul 17 06:17:10 UTC 2019 on sn-devel-184

By using talloc_get_parent() walking the entire talloc tree is
avoided.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---

diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index f4934917c7c..45faa0912ec 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -698,6 +698,8 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
 {
 	int ret;
 	void *ptr;
+	void *schema_parent = NULL;
+	bool is_already_parent;
 	struct dsdb_schema *old_schema;
 	old_schema = ldb_get_opaque(ldb, "dsdb_schema");
 	ret = ldb_set_opaque(ldb, "dsdb_schema", schema);
@@ -710,8 +712,9 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema,
 	talloc_unlink(ldb, old_schema);
 
 	/* Reference schema on ldb if it wasn't done already */
-	ret = talloc_is_parent(ldb, schema);
-	if (ret == 0) {
+	schema_parent = talloc_parent(schema);
+	is_already_parent = (schema_parent == ldb);
+	if (!is_already_parent) {
 		ptr = talloc_reference(ldb, schema);
 		if (ptr == NULL) {
 			return ldb_oom(ldb);
@@ -775,10 +778,10 @@ int dsdb_set_global_schema(struct ldb_context *ldb)
 	/* Don't write indices and attributes, it's expensive */
 	ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, SCHEMA_MEMORY_ONLY);
 	if (ret == LDB_SUCCESS) {
-		/* If ldb doesn't have a reference to the schema, make one,
-		 * just in case the original copy is replaced */
-		ret = talloc_is_parent(ldb, global_schema);
-		if (ret == 0) {
+		void *schema_parent = talloc_parent(global_schema);
+		bool is_already_parent =
+			(schema_parent == ldb);
+		if (!is_already_parent) {
 			ptr = talloc_reference(ldb, global_schema);
 			if (ptr == NULL) {
 				return ldb_oom(ldb);
@@ -809,7 +812,6 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
 	dsdb_schema_refresh_fn refresh_fn;
 	struct ldb_module *loaded_from_module;
 	bool use_global_schema;
-	int ret;
 	TALLOC_CTX *tmp_ctx = talloc_new(reference_ctx);
 	if (tmp_ctx == NULL) {
 		return NULL;
@@ -860,13 +862,28 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen
 	/* This removes the extra reference above */
 	talloc_free(tmp_ctx);
 
-	/* If ref ctx exists and doesn't already reference schema, then add
-	 * a reference.  Otherwise, just return schema.*/
-	ret = talloc_is_parent(reference_ctx, schema_out);
-	if ((ret == 1) || (!reference_ctx)) {
+	/*
+	 * If ref ctx exists and doesn't already reference schema, then add
+	 * a reference.  Otherwise, just return schema.
+	 *
+	 * We must use talloc_parent(), which is not quite free (there
+	 * is no direct parent pointer in talloc, only one on the
+	 * first child within a linked list), but is much cheaper than
+	 * talloc_is_parent() which walks the whole tree up to the top
+	 * looking for a potential grand-grand(etc)-parent.
+	 */
+	if (reference_ctx == NULL) {
 		return schema_out;
 	} else {
-		return talloc_reference(reference_ctx, schema_out);
+		void *schema_parent = talloc_parent(schema_out);
+		bool is_already_parent =
+			schema_parent == reference_ctx;
+		if (is_already_parent) {
+			return schema_out;
+		} else {
+			return talloc_reference(reference_ctx,
+						schema_out);
+		}
 	}
 }