From 6eabad9c9d977c1c5c6ecf7494a0be42ad113d23 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 29 Nov 2005 12:34:03 +0000 Subject: [PATCH] r11958: - fixed memory leaks in the ldb_result handling in ldb operations - removed an unnecessary level of pointer in ldb_search structure (This used to be commit b8d4afb14a18dfd8bac79882a035e74d3ed312bd) --- source4/dsdb/samdb/ldb_modules/proxy.c | 6 +- source4/dsdb/samdb/ldb_modules/rootdse.c | 9 +- source4/ldap_server/ldap_simple_ldb.c | 4 +- source4/lib/ldb/common/ldb.c | 129 ++++++++++++----------- source4/lib/ldb/include/ldb.h | 2 +- source4/lib/ldb/ldb_ildap/ldb_ildap.c | 2 +- source4/lib/ldb/ldb_tdb/ldb_tdb.c | 2 +- source4/lib/ldb/modules/ldb_map.c | 46 ++++---- source4/lib/ldb/modules/operational.c | 8 +- 9 files changed, 108 insertions(+), 100 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/proxy.c b/source4/dsdb/samdb/ldb_modules/proxy.c index 643ff5f3d0e..a567db689d3 100644 --- a/source4/dsdb/samdb/ldb_modules/proxy.c +++ b/source4/dsdb/samdb/ldb_modules/proxy.c @@ -295,14 +295,14 @@ static int proxy_search_bytree(struct ldb_module *module, struct ldb_request *re return -1; } - for (i = 0; i < (*newreq.op.search.res)->count; i++) { + for (i = 0; i < newreq.op.search.res->count; i++) { struct ldb_ldif ldif; printf("# record %d\n", i+1); - proxy_convert_record(module, (*newreq.op.search.res)->msgs[i]); + proxy_convert_record(module, newreq.op.search.res->msgs[i]); ldif.changetype = LDB_CHANGETYPE_NONE; - ldif.msg = (*newreq.op.search.res)->msgs[i]; + ldif.msg = newreq.op.search.res->msgs[i]; } return ret; diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c index df12011d897..4032aee8b29 100644 --- a/source4/dsdb/samdb/ldb_modules/rootdse.c +++ b/source4/dsdb/samdb/ldb_modules/rootdse.c @@ -46,11 +46,11 @@ static int rootdse_add_dynamic(struct ldb_module *module, struct ldb_request *re /* this is gross, and will be removed when I change ldb_result not to be so pointer crazy :-) */ - if (s->res[0][0].msgs == NULL) { + if (s->res->msgs == NULL) { return LDB_SUCCESS; } - msg = s->res[0][0].msgs[0]; + msg = s->res->msgs[0]; msg->dn = ldb_dn_explode(msg, ""); @@ -74,8 +74,7 @@ failed: */ static int rootdse_search_bytree(struct ldb_module *module, struct ldb_request *req) { - struct ldb_request r = *req; - struct ldb_search *s = &r.op.search; + struct ldb_search *s = &req->op.search; int ret; TALLOC_CTX *tmp_ctx; @@ -97,7 +96,7 @@ static int rootdse_search_bytree(struct ldb_module *module, struct ldb_request * } /* grab the static contents of the record */ - ret = ldb_next_request(module, &r); + ret = ldb_next_request(module, req); req->op.search.res = s->res; diff --git a/source4/ldap_server/ldap_simple_ldb.c b/source4/ldap_server/ldap_simple_ldb.c index 481db6052d8..b9ef085002d 100644 --- a/source4/ldap_server/ldap_simple_ldb.c +++ b/source4/ldap_server/ldap_simple_ldb.c @@ -171,10 +171,10 @@ static NTSTATUS sldb_Search(struct ldapsrv_partition *partition, struct ldapsrv_ lreq.op.search.scope = scope; lreq.op.search.tree = r->tree; lreq.op.search.attrs = attrs; - lreq.op.search.res = &res; ret = ldb_request(samdb, &lreq); - talloc_steal(samdb, res); + + res = talloc_steal(samdb, lreq.op.search.res); if (ret == LDB_SUCCESS) { for (i = 0; i < res->count; i++) { diff --git a/source4/lib/ldb/common/ldb.c b/source4/lib/ldb/common/ldb.c index ea99bf3e8bc..3dc62fd4d63 100644 --- a/source4/lib/ldb/common/ldb.c +++ b/source4/lib/ldb/common/ldb.c @@ -192,29 +192,60 @@ static int ldb_op_finish(struct ldb_context *ldb, int status) /* start an ldb request autostarts a transacion if none active and the operation is not a search - returns -1 on errors. + returns LDB_ERR_* on errors. */ - int ldb_request(struct ldb_context *ldb, struct ldb_request *request) { - int status; + int status, started_transaction=0; + struct ldb_request *r; ldb_reset_err_string(ldb); + /* to allow ldb modules to assume they can use the request ptr + as a talloc context for the request, we have to copy the + structure here */ + r = talloc(ldb, struct ldb_request); + if (r == NULL) { + ldb_oom(ldb); + return LDB_ERR_OPERATIONS_ERROR; + } + + *r = *request; + + if (r->operation == LDB_REQ_SEARCH) { + r->op.search.res = NULL; + } + + /* start a transaction if needed */ if ((!ldb->transaction_active) && (request->operation == LDB_REQ_ADD || request->operation == LDB_REQ_MODIFY || request->operation == LDB_REQ_DELETE || request->operation == LDB_REQ_RENAME)) { - status = ldb_transaction_start(ldb); - if (status != LDB_SUCCESS) return status; + if (status != LDB_SUCCESS) { + talloc_free(r); + return status; + } + started_transaction = 1; + } - status = ldb->modules->ops->request(ldb->modules, request); + /* call the first module in the chain */ + status = ldb->modules->ops->request(ldb->modules, r); + + /* the search call is the only one that returns something + other than a status code. We steal the results into + the context of the ldb before freeing the request */ + if (request->operation == LDB_REQ_SEARCH) { + request->op.search.res = talloc_steal(ldb, r->op.search.res); + } + talloc_free(r); + + if (started_transaction) { return ldb_op_finish(ldb, status); } - return ldb->modules->ops->request(ldb->modules, request); + return status; } /* @@ -229,34 +260,30 @@ int ldb_search(struct ldb_context *ldb, const struct ldb_dn *base, enum ldb_scope scope, const char *expression, - const char * const *attrs, struct ldb_result **res) + const char * const *attrs, + struct ldb_result **res) { - struct ldb_request *request; + struct ldb_request request; struct ldb_parse_tree *tree; int ret; (*res) = NULL; - request = talloc(ldb, struct ldb_request); - if (request == NULL) { - ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory")); - return -1; - } - tree = ldb_parse_tree(ldb, expression); if (tree == NULL) { ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Unable to parse search expression")); return -1; } - request->operation = LDB_REQ_SEARCH; - request->op.search.base = base; - request->op.search.scope = scope; - request->op.search.tree = tree; - request->op.search.attrs = attrs; - request->op.search.res = res; + request.operation = LDB_REQ_SEARCH; + request.op.search.base = base; + request.op.search.scope = scope; + request.op.search.tree = tree; + request.op.search.attrs = attrs; + + ret = ldb_request(ldb, &request); - ret = ldb_request(ldb, request); + (*res) = request.op.search.res; talloc_free(tree); @@ -270,22 +297,16 @@ int ldb_search(struct ldb_context *ldb, int ldb_add(struct ldb_context *ldb, const struct ldb_message *message) { - struct ldb_request *request; + struct ldb_request request; int status; status = ldb_msg_sanity_check(message); if (status != LDB_SUCCESS) return status; - request = talloc(ldb, struct ldb_request); - if (request == NULL) { - ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory")); - return -1; - } + request.operation = LDB_REQ_ADD; + request.op.add.message = message; - request->operation = LDB_REQ_ADD; - request->op.add.message = message; - - return ldb_request(ldb, request); + return ldb_request(ldb, &request); } /* @@ -294,22 +315,16 @@ int ldb_add(struct ldb_context *ldb, int ldb_modify(struct ldb_context *ldb, const struct ldb_message *message) { - struct ldb_request *request; + struct ldb_request request; int status; status = ldb_msg_sanity_check(message); if (status != LDB_SUCCESS) return status; - request = talloc(ldb, struct ldb_request); - if (request == NULL) { - ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory")); - return -1; - } - - request->operation = LDB_REQ_MODIFY; - request->op.mod.message = message; + request.operation = LDB_REQ_MODIFY; + request.op.mod.message = message; - return ldb_request(ldb, request); + return ldb_request(ldb, &request); } @@ -318,18 +333,12 @@ int ldb_modify(struct ldb_context *ldb, */ int ldb_delete(struct ldb_context *ldb, const struct ldb_dn *dn) { - struct ldb_request *request; - - request = talloc(ldb, struct ldb_request); - if (request == NULL) { - ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory")); - return -1; - } + struct ldb_request request; - request->operation = LDB_REQ_DELETE; - request->op.del.dn = dn; + request.operation = LDB_REQ_DELETE; + request.op.del.dn = dn; - return ldb_request(ldb, request); + return ldb_request(ldb, &request); } /* @@ -337,19 +346,13 @@ int ldb_delete(struct ldb_context *ldb, const struct ldb_dn *dn) */ int ldb_rename(struct ldb_context *ldb, const struct ldb_dn *olddn, const struct ldb_dn *newdn) { - struct ldb_request *request; - - request = talloc(ldb, struct ldb_request); - if (request == NULL) { - ldb_set_errstring(ldb->modules, talloc_strdup(ldb, "Not Enough memory")); - return -1; - } + struct ldb_request request; - request->operation = LDB_REQ_RENAME; - request->op.rename.olddn = olddn; - request->op.rename.newdn = newdn; + request.operation = LDB_REQ_RENAME; + request.op.rename.olddn = olddn; + request.op.rename.newdn = newdn; - return ldb_request(ldb, request); + return ldb_request(ldb, &request); } diff --git a/source4/lib/ldb/include/ldb.h b/source4/lib/ldb/include/ldb.h index 86e6a021da5..2fdf40b3bc6 100644 --- a/source4/lib/ldb/include/ldb.h +++ b/source4/lib/ldb/include/ldb.h @@ -274,7 +274,7 @@ struct ldb_search { enum ldb_scope scope; struct ldb_parse_tree *tree; const char * const *attrs; - struct ldb_result **res; + struct ldb_result *res; }; struct ldb_add { diff --git a/source4/lib/ldb/ldb_ildap/ldb_ildap.c b/source4/lib/ldb/ldb_ildap/ldb_ildap.c index cc9b5e65dbb..e195ec24aa0 100644 --- a/source4/lib/ldb/ldb_ildap/ldb_ildap.c +++ b/source4/lib/ldb/ldb_ildap/ldb_ildap.c @@ -407,7 +407,7 @@ static int ildb_request(struct ldb_module *module, struct ldb_request *req) req->op.search.scope, req->op.search.tree, req->op.search.attrs, - req->op.search.res); + &req->op.search.res); case LDB_REQ_ADD: return ildb_add(module, req->op.add.message); diff --git a/source4/lib/ldb/ldb_tdb/ldb_tdb.c b/source4/lib/ldb/ldb_tdb/ldb_tdb.c index d1e60e0f5da..0de7a3e558a 100644 --- a/source4/lib/ldb/ldb_tdb/ldb_tdb.c +++ b/source4/lib/ldb/ldb_tdb/ldb_tdb.c @@ -743,7 +743,7 @@ static int ltdb_request(struct ldb_module *module, struct ldb_request *req) req->op.search.scope, req->op.search.tree, req->op.search.attrs, - req->op.search.res); + &req->op.search.res); case LDB_REQ_ADD: return ltdb_add(module, req->op.add.message); diff --git a/source4/lib/ldb/modules/ldb_map.c b/source4/lib/ldb/modules/ldb_map.c index 49261d155cc..b056521f9f6 100644 --- a/source4/lib/ldb/modules/ldb_map.c +++ b/source4/lib/ldb/modules/ldb_map.c @@ -786,7 +786,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) enum ldb_scope scope = req->op.search.scope; struct ldb_parse_tree *tree = req->op.search.tree; const char * const *attrs = req->op.search.attrs; - struct ldb_result **res = req->op.search.res; + struct ldb_result *res; struct ldb_request new_req; struct ldb_parse_tree *new_tree; struct ldb_dn *new_base; @@ -818,9 +818,11 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) new_req.op.search.scope = scope; new_req.op.search.tree = new_tree; new_req.op.search.attrs = newattrs; - new_req.op.search.res = &newres; + mpret = ldb_request(privdat->mapped_ldb, req); + newres = new_req.op.search.res; + talloc_free(new_base); talloc_free(new_tree); talloc_free(newattrs); @@ -835,9 +837,10 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) - test if (full expression) is now true */ - *res = talloc(module, struct ldb_result); - (*res)->msgs = talloc_array(module, struct ldb_message *, newres->count); - (*res)->count = newres->count; + res = talloc(module, struct ldb_result); + req->op.search.res = res; + res->msgs = talloc_array(module, struct ldb_message *, newres->count); + res->count = newres->count; ret = 0; @@ -860,10 +863,11 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) mergereq.op.search.scope = LDB_SCOPE_BASE; mergereq.op.search.tree = ldb_parse_tree(module, ""); mergereq.op.search.attrs = NULL; - mergereq.op.search.res = &extrares; extraret = ldb_next_request(module, &mergereq); + extrares = mergereq.op.search.res; + if (extraret == -1) { ldb_debug(module->ldb, LDB_DEBUG_ERROR, "Error searching for extra data!\n"); } else if (extraret > 1) { @@ -883,7 +887,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) } if (ldb_match_msg(module->ldb, merged, tree, base, scope) != 0) { - (*res)->msgs[ret] = merged; + res->msgs[ret] = merged; ret++; } else { ldb_debug(module->ldb, LDB_DEBUG_TRACE, "Discarded merged message because it did not match"); @@ -892,7 +896,7 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) talloc_free(newres); - (*res)->count = ret; + res->count = ret; return LDB_SUCCESS; } @@ -903,39 +907,41 @@ static int map_search_mp(struct ldb_module *module, struct ldb_request *req) static int map_search_bytree(struct ldb_module *module, struct ldb_request *req) { const struct ldb_dn *base = req->op.search.base; - struct ldb_result **res = req->op.search.res; - struct ldb_result *fbres, *mpres = NULL; + struct ldb_result *fbres, *mpres, *res; int i, ret; - req->op.search.res = &fbres; ret = map_search_fb(module, req); - req->op.search.res = res; if (ret != LDB_SUCCESS) return ret; /* special dn's are never mapped.. */ if (ldb_dn_is_special(base)) { - *res = fbres; return ret; } - req->op.search.res = &mpres; + fbres = req->op.search.res; + ret = map_search_mp(module, req); - req->op.search.res = res; if (ret != LDB_SUCCESS) { return ret; } + mpres = req->op.search.res; + /* Merge results */ - *res = talloc(module, struct ldb_result); - (*res)->msgs = talloc_array(*res, struct ldb_message *, fbres->count + mpres->count); + res = talloc(module, struct ldb_result); + res->msgs = talloc_array(res, struct ldb_message *, fbres->count + mpres->count); ldb_debug(module->ldb, LDB_DEBUG_TRACE, "Merging %d mapped and %d fallback messages", mpres->count, fbres->count); - for (i = 0; i < fbres->count; i++) (*res)->msgs[i] = fbres->msgs[i]; - for (i = 0; i < mpres->count; i++) (*res)->msgs[fbres->count + i] = mpres->msgs[i]; + for (i = 0; i < fbres->count; i++) { + res->msgs[i] = talloc_steal(res->msgs, fbres->msgs[i]); + } + for (i = 0; i < mpres->count; i++) { + res->msgs[fbres->count + i] = talloc_steal(res->msgs, mpres->msgs[i]); + } - (*res)->count = fbres->count + mpres->count; + res->count = fbres->count + mpres->count; return LDB_SUCCESS; } diff --git a/source4/lib/ldb/modules/operational.c b/source4/lib/ldb/modules/operational.c index 7c554e8541b..2fdd255e991 100644 --- a/source4/lib/ldb/modules/operational.c +++ b/source4/lib/ldb/modules/operational.c @@ -182,7 +182,7 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque const char * const *attrs = req->op.search.attrs; const char **search_attrs = NULL; - *(req->op.search.res) = NULL; + req->op.search.res = NULL; /* replace any attributes in the parse tree that are searchable, but are stored using a different name in the @@ -225,8 +225,8 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque /* for each record returned post-process to add any derived attributes that have been asked for */ - for (r = 0; r < (*(req->op.search.res))->count; r++) { - if (operational_search_post_process(module, (*(req->op.search.res))->msgs[r], attrs) != 0) { + for (r = 0; r < req->op.search.res->count; r++) { + if (operational_search_post_process(module, req->op.search.res->msgs[r], attrs) != 0) { goto failed; } } @@ -237,7 +237,7 @@ static int operational_search_bytree(struct ldb_module *module, struct ldb_reque failed: talloc_free(search_attrs); - talloc_free(*(req->op.search.res)); + talloc_free(req->op.search.res); ldb_oom(module->ldb); return LDB_ERR_OTHER; } -- 2.34.1