From efad6a64030a7586a65e57697305141eddf63f81 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 6 Aug 2013 16:51:02 -0400 Subject: Renames and fix a memory leak Rename backend_staged_data to backend_staged_search. Fix some formatting. Change how we walk the list of entries retrieved using a staged search so that if the map's been removed since the search was staged, we still free the temporary entry structures. --- src/back-sch-nss.c | 40 ++++++++++++++++++------------------ src/back-sch.c | 59 +++++++++++++++++++++++++++--------------------------- src/back-sch.h | 12 +++++------ 3 files changed, 56 insertions(+), 55 deletions(-) (limited to 'src') diff --git a/src/back-sch-nss.c b/src/back-sch-nss.c index a45d42d..c8aaa20 100644 --- a/src/back-sch-nss.c +++ b/src/back-sch-nss.c @@ -465,20 +465,20 @@ repeat: return entries; } -/* Check filter for a component like (uid=) and if found, - * stage NSSWITCH lookup. Lookup will be performed later, with call to backend_retrieve_from_nsswitch */ +/* Check if the filter is one (like uid=) that should trigger an + * nsswitch lookup, and if it is, make a note that we should perform such a + * lookup. */ void backend_search_nsswitch(struct backend_set_data *set_data, struct backend_search_cbdata *cbdata) { int result, rc; struct backend_search_filter_config config = {FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, NULL}; - struct backend_staged_data *staged = NULL; + struct backend_staged_search *staged = NULL; char *idptr = NULL; unsigned long id; - /* There was no match but we asked to check NSSWITCH */ - /* First, we search the filter to see if it includes cn|uid= check */ + /* First, we search the filter to see if it includes a cn|uid= test. */ result = slapi_filter_apply(cbdata->filter, backend_search_filter_has_cn_uid, &config, &rc); if ((result != SLAPI_FILTER_SCAN_STOP)) { @@ -515,22 +515,21 @@ backend_search_nsswitch(struct backend_set_data *set_data, } } - - staged = malloc(sizeof(struct backend_staged_data)); + staged = malloc(sizeof(*staged)); if (staged == NULL) { goto fail; } staged->map_group = slapi_ch_strdup(set_data->common.group); staged->map_set = slapi_ch_strdup(set_data->common.set); - staged->set_data_fixup = NULL; + staged->set_data = NULL; staged->count = 0; staged->entries = NULL; staged->container_sdn = slapi_ch_strdup(slapi_sdn_get_dn(set_data->container_sdn)); staged->type = cbdata->check_nsswitch; - staged->name = config.name; + staged->name = config.name; /* takes ownership */ staged->is_id = config.search_gid || config.search_uid; staged->search_members = config.search_members; @@ -543,9 +542,10 @@ fail: return; } - /* perform look up against NSSWITCH and create entry based on that */ + /* Actually look up the information that we previously noted that we should, + * then convert whatever we find into one or more Slapi_Entry pointers. */ bool_t -backend_retrieve_from_nsswitch(struct backend_staged_data *staged, +backend_retrieve_from_nsswitch(struct backend_staged_search *staged, struct backend_search_cbdata *cbdata) { Slapi_Entry *entry, **entries; @@ -554,7 +554,7 @@ backend_retrieve_from_nsswitch(struct backend_staged_data *staged, if (((staged->type == SCH_NSSWITCH_GROUP) && staged->search_members) && (NULL != staged->name)) { entries = backend_retrieve_group_list_from_nsswitch(staged->name, staged->container_sdn, - cbdata, &staged->count); + cbdata, &staged->count); if (entries != NULL) { staged->entries = entries; for (i = 0; i < staged->count; i++) { @@ -567,11 +567,11 @@ backend_retrieve_from_nsswitch(struct backend_staged_data *staged, if ((staged->type == SCH_NSSWITCH_GROUP) && (NULL != staged->name)) { entry = backend_retrieve_group_entry_from_nsswitch(staged->name, staged->is_id, - staged->container_sdn, - cbdata); - if (entry) { + staged->container_sdn, + cbdata); + if (entry != NULL) { slapi_entry_add_string(entry, "schema-compat-origin", "nsswitch"); - staged->entries = malloc(sizeof(Slapi_Entry *)); + staged->entries = malloc(sizeof(staged->entries[0])); if (staged->entries != NULL) { staged->entries[0] = entry; staged->count = 1; @@ -585,11 +585,11 @@ backend_retrieve_from_nsswitch(struct backend_staged_data *staged, if ((staged->type == SCH_NSSWITCH_USER) && (NULL != staged->name)) { entry = backend_retrieve_user_entry_from_nsswitch(staged->name, staged->is_id, - staged->container_sdn, - cbdata); - if (entry) { + staged->container_sdn, + cbdata); + if (entry != NULL) { slapi_entry_add_string(entry, "schema-compat-origin", "nsswitch"); - staged->entries = malloc(sizeof(Slapi_Entry *)); + staged->entries = malloc(sizeof(staged->entries[0])); if (staged->entries != NULL) { staged->entries[0] = entry; staged->count = 1; diff --git a/src/back-sch.c b/src/back-sch.c index 63ee110..8359056 100644 --- a/src/back-sch.c +++ b/src/back-sch.c @@ -155,7 +155,8 @@ backend_set_config_read_config(struct plugin_state *state, Slapi_Entry *e, const char *group, const char *container, bool_t *flag, struct backend_shr_set_data **pret) { - char **bases, *entry_filter, **attributes, *rdn_format, *dn, *nsswitch_min_id, *check_nsswitch, *strp; + char **bases, *entry_filter, **attributes, *rdn_format, *dn; + char *nsswitch_min_id, *check_nsswitch, *strp; bool_t check_access; struct backend_set_data ret; Slapi_DN *tmp_sdn; @@ -238,7 +239,8 @@ backend_set_config_read_config(struct plugin_state *state, Slapi_Entry *e, /* If we're adding nsswitch-based entries to this map, make * sure that we copy the schema-compat-origin and SID * attributes, so that we can read the former during the BIND - * callback. */ + * callback. FIXME: store that in the entry's backend_data to + * avoid surprising clients. */ backend_shr_add_strlist(&ret.attribute_format, "objectClass=extensibleObject"); backend_shr_add_strlist(&ret.attribute_format, "schema-compat-origin=%{schema-compat-origin}"); backend_shr_add_strlist(&ret.attribute_format, "ipaNTSecurityIdentifier=%{ipaNTSecurityIdentifier}"); @@ -1005,7 +1007,7 @@ backend_search_set_cb(const char *group, const char *set, bool_t flag, struct backend_set_data *set_data; Slapi_Entry *set_entry; int result, n_entries; - int n_entries_nsswitch; + int n_entries_without_nsswitch; const char *ndn; cbdata = cb_data; @@ -1013,9 +1015,10 @@ backend_search_set_cb(const char *group, const char *set, bool_t flag, cbdata->check_access = set_data->check_access; cbdata->check_nsswitch = set_data->check_nsswitch; cbdata->nsswitch_min_id = set_data->nsswitch_min_id; - /* If any entries were actually returned by the descending callback, - * avoid to look up in nsswitch even if this set is marked to look up */ - n_entries_nsswitch = cbdata->n_entries; + + /* Count the number of results that we've found before looking at this + * set of entries. */ + n_entries_without_nsswitch = cbdata->n_entries; /* Check the set itself, unless it's also the group, in which case we * already evaluated it for this search. */ @@ -1071,14 +1074,13 @@ backend_search_set_cb(const char *group, const char *set, bool_t flag, map_data_foreach_entry_id(cbdata->state, group, set, NULL, backend_search_entry_cb, cbdata); #ifdef USE_NSSWITCH - /* If we didn't find an exact match for the entry but asked to look up NSSWITCH, - * then try to search NSSWITCH. If search filters would match, the search will be - * staged for retrieval. The retrieval process is run after lookup is completed - * for all maps as we need to ensure there is no contention for the global - * map cache lock. The contention might occur if NSSWITCH would need to re-kinit - * its kerberos credentials -- this would cause changes in the original LDAP tree - * which, in turn, will trigger modification of the map cache entries. */ - if ((n_entries_nsswitch == cbdata->n_entries) && + /* If we didn't find a matching entry in this set, but we're + * configured to also consult nsswitch, check if the search + * filter is one that should trigger an nsswitch lookup, and + * make a note if it would. We'll come back and actually + * perform the lookup later when we're not holding a lock that + * can stall other threads. */ + if ((cbdata->n_entries == n_entries_without_nsswitch) && (cbdata->check_nsswitch != SCH_NSSWITCH_NONE)) { backend_search_nsswitch(set_data, cbdata); } @@ -1109,7 +1111,7 @@ backend_search_find_set_data_in_group_cb(const char *group, const char *set, boo if ((0 == strcmp(group, cbdata->cur_staged->map_group)) && (0 == strcmp(set, cbdata->cur_staged->map_set))) { - cbdata->cur_staged->set_data_fixup = set_data; + cbdata->cur_staged->set_data = set_data; } return TRUE; @@ -1123,7 +1125,7 @@ backend_search_find_set_data_cb(const char *group, void *cb_data) cbdata = cb_data; map_data_foreach_map(cbdata->state, group, - backend_search_find_set_data_in_group_cb, cbdata); + backend_search_find_set_data_in_group_cb, cb_data); return TRUE; } @@ -1230,7 +1232,7 @@ static int backend_search_cb(Slapi_PBlock *pb) { struct backend_search_cbdata cbdata; - struct backend_staged_data *staged, *next; + struct backend_staged_search *staged, *next; int i; if (wrap_get_call_level() > 0) { @@ -1298,7 +1300,7 @@ backend_search_cb(Slapi_PBlock *pb) /* Go over the list of staged requests and retrieve entries. * It is important to perform the retrieval *without* holding any locks to the map cache */ staged = cbdata.staged; - while (staged) { + while (staged != NULL) { if (staged->entries == NULL) { backend_retrieve_from_nsswitch(staged, &cbdata); } @@ -1309,22 +1311,21 @@ backend_search_cb(Slapi_PBlock *pb) /* Add the entries to the map cache */ wrap_inc_call_level(); map_wrlock(); - while (staged) { + while (staged != NULL) { if (staged->entries) { cbdata.cur_staged = staged; /* We actually need to find the original set first */ map_data_foreach_domain(cbdata.state, backend_search_find_set_data_cb, &cbdata); - if (cbdata.cur_staged->set_data_fixup != NULL) { - for (i = 0; i < staged->count ; i++) { - if (staged->entries[i] != NULL) { - if (!map_data_check_entry(cbdata.state, - staged->map_group, staged->map_set, - slapi_sdn_get_ndn(staged->entries[i]))) { - backend_set_entry(cbdata.pb, staged->entries[i], staged->set_data_fixup); - } - slapi_entry_free(staged->entries[i]); - staged->entries[i] = NULL; + for (i = 0; i < staged->count ; i++) { + if (staged->entries[i] != NULL) { + if ((cbdata.cur_staged->set_data != NULL) && + !map_data_check_entry(cbdata.state, + staged->map_group, staged->map_set, + slapi_sdn_get_ndn(staged->entries[i]))) { + backend_set_entry(cbdata.pb, staged->entries[i], staged->set_data); } + slapi_entry_free(staged->entries[i]); + staged->entries[i] = NULL; } } free(staged->entries); diff --git a/src/back-sch.h b/src/back-sch.h index 3b91c66..7887127 100644 --- a/src/back-sch.h +++ b/src/back-sch.h @@ -45,10 +45,10 @@ struct backend_entry_data { Slapi_Entry *e; }; -struct backend_staged_data { - struct backend_staged_data *next; - struct backend_set_data *set_data_fixup; +struct backend_staged_search { + struct backend_staged_search *next; char *map_group, *map_set; + struct backend_set_data *set_data; enum sch_search_nsswitch_t type; bool_t is_id; bool_t search_members; @@ -79,14 +79,14 @@ struct backend_search_cbdata { bool_t matched; char *closest_match, *text; int n_entries; - struct backend_staged_data *staged; - struct backend_staged_data *cur_staged; + struct backend_staged_search *staged; + struct backend_staged_search *cur_staged; }; void backend_search_nsswitch(struct backend_set_data *set_data, struct backend_search_cbdata *cbdata); -bool_t backend_retrieve_from_nsswitch(struct backend_staged_data *staged, +bool_t backend_retrieve_from_nsswitch(struct backend_staged_search *staged, struct backend_search_cbdata *cbdata); int backend_sch_do_pam_auth(Slapi_PBlock *pb, const char *username); -- cgit