[SCM] Samba Shared Repository - branch master updated

Andrew Tridgell tridge at samba.org
Fri Dec 4 00:47:48 MST 2009


The branch, master has been updated
       via  be78d4a... s4-ldb: fixed show_deleted module not to corrupt parse trees
       via  ced3eef... s4-drsutil: fixed a memory leak in samdb_search_count
      from  4f6d5d0... s4 torture: Convert create_complex_file to use BASIC_INFO instead of deprecated command

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit be78d4a70de2aede3b9c4a644ce64c85141790cb
Author: Andrew Tridgell <tridge at samba.org>
Date:   Fri Dec 4 17:46:14 2009 +1100

    s4-ldb: fixed show_deleted module not to corrupt parse trees
    
    The show_deleted module was using a static private ptr in the module
    to hold a parse tree to save on parsing. The code caused this
    static ptr to change with each search, which caused incorrect
    searches and numerous valgrind errors.
    
    This patch replaces it with a hand-built parse tree.

commit ced3eef776dd44d0f3e9219f77e2660f9e49fa92
Author: Andrew Tridgell <tridge at samba.org>
Date:   Fri Dec 4 17:45:38 2009 +1100

    s4-drsutil: fixed a memory leak in samdb_search_count
    
    In general functions that don't return any memory should not take a memory context.
    Otherwise it is too easy to have a bug like this where memory is leaked

-----------------------------------------------------------------------

Summary of changes:
 source4/dsdb/common/util.c                    |    9 +++--
 source4/dsdb/samdb/ldb_modules/operational.c  |    5 ++-
 source4/dsdb/samdb/ldb_modules/show_deleted.c |   49 +++++++++++++------------
 source4/rpc_server/samr/dcesrv_samr.c         |    6 ++--
 4 files changed, 38 insertions(+), 31 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index feebab8..8c9c982 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -187,18 +187,19 @@ struct dom_sid *samdb_search_dom_sid(struct ldb_context *sam_ldb,
   return the count of the number of records in the sam matching the query
 */
 int samdb_search_count(struct ldb_context *sam_ldb,
-		       TALLOC_CTX *mem_ctx,
 		       struct ldb_dn *basedn,
-		       const char *format, ...) _PRINTF_ATTRIBUTE(4,5)
+		       const char *format, ...) _PRINTF_ATTRIBUTE(3,4)
 {
 	va_list ap;
 	struct ldb_message **res;
-	const char * const attrs[] = { NULL };
+	const char *attrs[] = { NULL };
 	int ret;
+	TALLOC_CTX *tmp_ctx = talloc_new(sam_ldb);
 
 	va_start(ap, format);
-	ret = gendb_search_v(sam_ldb, mem_ctx, basedn, &res, attrs, format, ap);
+	ret = gendb_search_v(sam_ldb, tmp_ctx, basedn, &res, attrs, format, ap);
 	va_end(ap);
+	talloc_free(tmp_ctx);
 
 	return ret;
 }
diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
index 031544d..cc29476 100644
--- a/source4/dsdb/samdb/ldb_modules/operational.c
+++ b/source4/dsdb/samdb/ldb_modules/operational.c
@@ -104,7 +104,10 @@ static int construct_primary_group_token(struct ldb_module *module,
 
 	ldb = ldb_module_get_ctx(module);
 
-	if (samdb_search_count(ldb, ldb, msg->dn, "(objectclass=group)") == 1) {
+	/* this is horrendously inefficient! we're doing a subtree
+	 * search for every DN we return. So that's N^2 in the
+	 * total number of objects! */
+	if (samdb_search_count(ldb, msg->dn, "(objectclass=group)") == 1) {
 		primary_group_token
 			= samdb_result_rid_from_sid(ldb, msg, "objectSid", 0);
 		return samdb_msg_add_int(ldb, ldb, msg, "primaryGroupToken",
diff --git a/source4/dsdb/samdb/ldb_modules/show_deleted.c b/source4/dsdb/samdb/ldb_modules/show_deleted.c
index 557e36f..11503e6 100644
--- a/source4/dsdb/samdb/ldb_modules/show_deleted.c
+++ b/source4/dsdb/samdb/ldb_modules/show_deleted.c
@@ -40,7 +40,6 @@ static int show_deleted_search(struct ldb_module *module, struct ldb_request *re
 	struct ldb_control *control;
 	struct ldb_control **saved_controls;
 	struct ldb_request *down_req;
-	struct ldb_parse_tree *nodeleted_tree;
 	struct ldb_parse_tree *new_tree = req->op.search.tree;
 	int ret;
 
@@ -50,19 +49,33 @@ static int show_deleted_search(struct ldb_module *module, struct ldb_request *re
 	control = ldb_request_get_control(req, LDB_CONTROL_SHOW_DELETED_OID);
 
 	if (! control) {
-		nodeleted_tree = talloc_get_type(ldb_module_get_private(module), 
-						 struct ldb_parse_tree);
-		if (nodeleted_tree) {
-			new_tree = talloc(req, struct ldb_parse_tree);
-			if (!new_tree) {
-				ldb_oom(ldb);
-				return LDB_ERR_OPERATIONS_ERROR;
-			}
-			*new_tree = *nodeleted_tree;
-			/* Replace dummy part of 'and' with the old, tree,
-			   without a parse step */
-			new_tree->u.list.elements[0] = req->op.search.tree;
+		/* FIXME: we could use a constant tree here once we
+		   are sure that no ldb modules modify trees
+		   in-situ */
+		new_tree = talloc(req, struct ldb_parse_tree);
+		if (!new_tree) {
+			ldb_oom(ldb);
+			return LDB_ERR_OPERATIONS_ERROR;
 		}
+		new_tree->operation = LDB_OP_AND;
+		new_tree->u.list.num_elements = 2;
+		new_tree->u.list.elements = talloc_array(new_tree, struct ldb_parse_tree *, 2);
+		if (!new_tree->u.list.elements) {
+			ldb_oom(ldb);
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		new_tree->u.list.elements[0] = talloc(new_tree->u.list.elements, struct ldb_parse_tree);
+		new_tree->u.list.elements[0]->operation = LDB_OP_NOT;
+		new_tree->u.list.elements[0]->u.isnot.child =
+			talloc(new_tree->u.list.elements, struct ldb_parse_tree);
+		if (!new_tree->u.list.elements[0]->u.isnot.child) {
+			ldb_oom(ldb);
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
+		new_tree->u.list.elements[0]->u.isnot.child->operation = LDB_OP_EQUALITY;
+		new_tree->u.list.elements[0]->u.isnot.child->u.equality.attr = "isDeleted";
+		new_tree->u.list.elements[0]->u.isnot.child->u.equality.value = data_blob_string_const("TRUE");
+		new_tree->u.list.elements[1] = req->op.search.tree;
 	}
 	
 	ret = ldb_build_search_req_ex(&down_req, ldb, req,
@@ -89,20 +102,10 @@ static int show_deleted_search(struct ldb_module *module, struct ldb_request *re
 static int show_deleted_init(struct ldb_module *module)
 {
 	struct ldb_context *ldb;
-	struct ldb_parse_tree *nodeleted_tree;
 	int ret;
 
 	ldb = ldb_module_get_ctx(module);
 
-	nodeleted_tree = ldb_parse_tree(module, "(&(replace=me)(!(isDeleted=TRUE)))");
-	if (!nodeleted_tree) {
-		ldb_debug(ldb, LDB_DEBUG_ERROR,
-			"show_deleted: Unable to parse isDeleted master expression!\n");
-		return LDB_ERR_OPERATIONS_ERROR;
-	}
-
-	ldb_module_set_private(module, nodeleted_tree);
-
 	ret = ldb_mod_register_control(module, LDB_CONTROL_SHOW_DELETED_OID);
 	if (ret != LDB_SUCCESS) {
 		ldb_debug(ldb, LDB_DEBUG_ERROR,
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 725ecba..1621003 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -518,12 +518,12 @@ static NTSTATUS dcesrv_samr_info_DomGeneralInformation(struct samr_domain_state
 	}
 
 	/* No users in BUILTIN, and the LOCAL group types are only in builtin, and the global group type is never in BUILTIN */
-	info->num_users = samdb_search_count(state->sam_ctx, mem_ctx, state->domain_dn, 
+	info->num_users = samdb_search_count(state->sam_ctx, state->domain_dn,
 					     "(objectClass=user)");
-	info->num_groups = samdb_search_count(state->sam_ctx, mem_ctx, state->domain_dn,
+	info->num_groups = samdb_search_count(state->sam_ctx, state->domain_dn,
 					      "(&(objectClass=group)(sAMAccountType=%u))",
 					      ATYPE_GLOBAL_GROUP);
-	info->num_aliases = samdb_search_count(state->sam_ctx, mem_ctx, state->domain_dn,
+	info->num_aliases = samdb_search_count(state->sam_ctx, state->domain_dn,
 					       "(&(objectClass=group)(sAMAccountType=%u))",
 					       ATYPE_LOCAL_GROUP);
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list