From ced3eef776dd44d0f3e9219f77e2660f9e49fa92 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 4 Dec 2009 17:45:38 +1100 Subject: [PATCH] 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 --- source4/dsdb/common/util.c | 9 +++++---- source4/dsdb/samdb/ldb_modules/operational.c | 5 ++++- source4/rpc_server/samr/dcesrv_samr.c | 6 +++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index feebab8d459..8c9c98201b3 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 031544d6a81..cc294766657 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/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c index 725ecba21f8..1621003ea3d 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); -- 2.34.1