From d4eedec8e22e8973b71f560c4f2ab1232abbd91b Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Feb 06 2012 21:51:07 +0000 Subject: Trac Ticket 51 - memory leaks in 389-ds-base-1.2.8.2-1.el5? https://fedorahosted.org/389/ticket/51 Fix description: Ran valgrind with the MMR+SASL servers and fixed leaks found in the test. [plugin/replication/repl5_connection.c] conn_connect could have overridden conn->ld without releasing it. This patch releases it if necessary. [slapd/dn.c] If DN normalization fails in slapi_sdn_get_dn, this patch releases the locally strdup'ed string. [slapd/modify.c, modutil.c] DN syntax attribute value is found in mods, it was normalized and replaced in slapi_mods_insert_at. It leaked the pre- noralized value. Instead, this patch normalizes mods in do_modify and frees it when the modify is done. [slapd/operation.c] modrdn_newsuperior_address.sdn was not release when the modrdn operaton is done. This patch adds the release code. --- diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c index 4cf69d1..a074a9f 100644 --- a/ldap/servers/plugins/replication/repl5_connection.c +++ b/ldap/servers/plugins/replication/repl5_connection.c @@ -1085,6 +1085,11 @@ conn_connect(Repl_Connection *conn) secure ? "secure" : "non-secure", (secure == 2) ? " startTLS" : ""); /* shared = 1 because we will read results from a second thread */ + if (conn->ld) { + /* Since we call slapi_ldap_init, we must call slapi_ldap_unbind */ + /* ldap_unbind internally calls ldap_ld_free */ + slapi_ldap_unbind(conn->ld); + } conn->ld = slapi_ldap_init_ext(NULL, conn->hostname, conn->port, secure, 1, NULL); if (NULL == conn->ld) { @@ -1176,8 +1181,7 @@ close_connection_internal(Repl_Connection *conn) to use conn->ld */ if (NULL != conn->ld) { - /* Since we call slapi_ldap_init, - we must call slapi_ldap_unbind */ + /* Since we call slapi_ldap_init, we must call slapi_ldap_unbind */ slapi_ldap_unbind(conn->ld); } conn->ld = NULL; diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c index aeb24b3..568871f 100644 --- a/ldap/servers/slapd/dn.c +++ b/ldap/servers/slapd/dn.c @@ -2256,8 +2256,9 @@ slapi_sdn_get_dn(const Slapi_DN *sdn) ncsdn->flag = slapi_setbit_uchar(sdn->flag, FLAG_DN); PR_INCREMENT_COUNTER(slapi_sdn_counter_dn_created); PR_INCREMENT_COUNTER(slapi_sdn_counter_dn_exist); + } else { /* else (rc < 0); normalization failed. return NULL */ + slapi_ch_free_string(&udn); } - /* else (rc < 0); normzlization failed. return NULL */ return sdn->dn; } else { return NULL; diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 563512d..bffc788 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -142,6 +142,7 @@ do_modify( Slapi_PBlock *pb ) char *old_pw = NULL; /* remember the old password */ char *rawdn = NULL; int minssf_exclude_rootdse = 0; + LDAPMod **normalized_mods = NULL; LDAPDebug( LDAP_DEBUG_TRACE, "do_modify\n", 0, 0, 0 ); @@ -389,12 +390,22 @@ do_modify( Slapi_PBlock *pb ) mods = slapi_mods_get_ldapmods_passout (&smods); - slapi_pblock_set( pb, SLAPI_MODIFY_MODS, mods); + /* normalize the mods */ + normalized_mods = normalize_mods2bvals((const LDAPMod**)mods); + ldap_mods_free (mods, 1 /* Free the Array and the Elements */); + if (normalized_mods == NULL) { + op_shared_log_error_access(pb, "MOD", rawdn?rawdn:"", + "mod includes invalid dn format"); + send_ldap_result(pb, LDAP_INVALID_DN_SYNTAX, NULL, + "mod includes invalid dn format", 0, NULL); + goto free_and_return; + } + slapi_pblock_set(pb, SLAPI_MODIFY_MODS, normalized_mods); op_shared_modify ( pb, pw_change, old_pw ); - slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); - ldap_mods_free (mods, 1 /* Free the Array and the Elements */); + slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &normalized_mods); + ldap_mods_free (normalized_mods, 1 /* Free the Array and the Elements */); free_and_return:; slapi_ch_free ((void**)&rawdn); diff --git a/ldap/servers/slapd/modutil.c b/ldap/servers/slapd/modutil.c index 8216c07..18b7fb6 100644 --- a/ldap/servers/slapd/modutil.c +++ b/ldap/servers/slapd/modutil.c @@ -188,7 +188,6 @@ void slapi_mods_insert_at(Slapi_Mods *smods, LDAPMod *mod, int pos) { int i; - Slapi_Attr a = {0}; if (NULL == mod) { return; @@ -198,18 +197,6 @@ slapi_mods_insert_at(Slapi_Mods *smods, LDAPMod *mod, int pos) { smods->mods[i+1]= smods->mods[i]; } - slapi_attr_init(&a, mod->mod_type); - /* Check if the type of the to-be-added values has DN syntax or not. */ - if (slapi_attr_is_dn_syntax_attr(&a)) { - struct berval **mbvp = NULL; - for (mbvp = mod->mod_bvalues; mbvp && *mbvp; mbvp++) { - Slapi_DN *sdn = slapi_sdn_new_dn_byref((*mbvp)->bv_val); - (*mbvp)->bv_val = slapi_ch_strdup(slapi_sdn_get_dn(sdn)); - (*mbvp)->bv_len = slapi_sdn_get_ndn_len(sdn); - slapi_sdn_free(&sdn); - } - } - attr_done(&a); smods->mods[pos]= mod; smods->num_mods++; smods->mods[smods->num_mods]= NULL; diff --git a/ldap/servers/slapd/operation.c b/ldap/servers/slapd/operation.c index 1479ee9..f5b1627 100644 --- a/ldap/servers/slapd/operation.c +++ b/ldap/servers/slapd/operation.c @@ -481,6 +481,7 @@ operation_parameters_done (struct slapi_operation_parameters *sop) case SLAPI_OPERATION_MODRDN: slapi_ch_free((void **)&(sop->p.p_modrdn.modrdn_newrdn)); slapi_ch_free((void **)&(sop->p.p_modrdn.modrdn_newsuperior_address.uniqueid)); + slapi_sdn_free(&sop->p.p_modrdn.modrdn_newsuperior_address.sdn); ldap_mods_free(sop->p.p_modrdn.modrdn_mods, 1 /* Free the Array and the Elements */); sop->p.p_modrdn.modrdn_mods= NULL; break; diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c index df3f7ef..16ba40e 100644 --- a/ldap/servers/slapd/search.c +++ b/ldap/servers/slapd/search.c @@ -407,7 +407,7 @@ do_search( Slapi_PBlock *pb ) } free_and_return:; - if ( !psearch || rc < 0 ) { + if ( !psearch || rc != 0 ) { slapi_ch_free_string(&fstr); slapi_filter_free( filter, 1 ); charray_free( attrs ); /* passing NULL is fine */