Browse Source

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.
Noriko Hosoi 13 years ago
parent
commit
d4eedec8e2

+ 6 - 2
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;

+ 2 - 1
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;

+ 14 - 3
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);

+ 0 - 13
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;

+ 1 - 0
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;

+ 1 - 1
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 */