From 6954ce25351d8dd2c88d57b51d3b91900e3de605 Mon Sep 17 00:00:00 2001

From: Rich Megginson <rmeggins@redhat.com>

Date: Thu, 5 Sep 2013 19:45:44 -0600

Subject: [PATCH 4/4] Ticket #47455 - valgrind - value mem leaks, uninit mem usage

https://fedorahosted.org/389/ticket/47455

Reviewed by: nkinder (Thanks!)

Branch: 389-ds-base-1.3.1

Fix Description: The problem was that slapi_valueset_add_attr_valuearray_ext

was assuming the caller was going to free the entire given vs upon failure.

This is fine for the value replace case but not for the add 1 value case.

Callers of slapi_valueset_add_attr_valuearray_ext must provide

the dup_index parameter if using SLAPI_VALUE_FLAG_PASSIN and

SLAPI_VALUE_FLAG_DUPCHECK, and if there is more than one value.  The caller

needs to know which of the values from addvals is in vs to properly clean up

with no memory leaks.

Platforms tested: RHEL6 x86_64

Flag Day: no

Doc impact: no

(cherry picked from commit 3adc242bcc8c6d0d05d5d9773f32b4f81afb6e6d)

(cherry picked from commit 6357ced2e4380def053966e849eac45e44009662)

---

 ldap/servers/slapd/slapi-private.h |    5 +++++

 ldap/servers/slapd/valueset.c      |   13 +++++++++----

 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h

index b2364ab..194f3fd 100644

--- a/ldap/servers/slapd/slapi-private.h

+++ b/ldap/servers/slapd/slapi-private.h

@@ -846,6 +846,11 @@ void valuearray_add_valuearray_fast( Slapi_Value ***vals, Slapi_Value **addvals,

 Slapi_Value * valueset_find_sorted (const Slapi_Attr *a, const Slapi_ValueSet *vs, const Slapi_Value *v, int *index);

 int valueset_insert_value_to_sorted(const Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *vi, int dupcheck);

 void valueset_array_to_sorted (const Slapi_Attr *a, Slapi_ValueSet *vs);

+/* NOTE: if the flags include SLAPI_VALUE_FLAG_PASSIN and SLAPI_VALUE_FLAG_DUPCHECK

+ * THE CALLER MUST PROVIDE THE dup_index PARAMETER in order to know where in addval

+ * the un-copied values start e.g. to free them for cleanup

+ * see valueset_replace_valuearray_ext() for an example

+ */

 int slapi_valueset_add_attr_valuearray_ext(const Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value **addval, int nvals, unsigned long flags, int *dup_index);

 int valuearray_find(const Slapi_Attr *a, Slapi_Value **va, const Slapi_Value *v);

 int valuearray_dn_normalize_value(Slapi_Value **vals);

diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c

index 85d2274..8a4bd03 100644

--- a/ldap/servers/slapd/valueset.c

+++ b/ldap/servers/slapd/valueset.c

@@ -1134,8 +1134,9 @@ slapi_valueset_add_attr_valuearray_ext(const Slapi_Attr *a, Slapi_ValueSet *vs,

 					rc = LDAP_TYPE_OR_VALUE_EXISTS;

 					if (dup_index) *dup_index = i;

 					if (passin) {

-						/* we have to NULL out the first value so valuearray_free won't delete values in addvals */

-						(vs->va)[0] = NULL;

+						PR_ASSERT((i == 0) || dup_index);

+						/* caller must provide dup_index to know how far we got in addvals */

+						(vs->va)[vs->num] = NULL;

 					} else {

 						slapi_value_free(&(vs->va)[vs->num]);

 					}

@@ -1379,8 +1380,9 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *

     } else {

 	/* verify the given values are not duplicated.  */

 	unsigned long flags = SLAPI_VALUE_FLAG_PASSIN|SLAPI_VALUE_FLAG_DUPCHECK;

+	int dupindex = 0;

 	Slapi_ValueSet *vs_new = slapi_valueset_new();

-	rc = slapi_valueset_add_attr_valuearray_ext (a, vs_new, valstoreplace, vals_count, flags, NULL);

+	rc = slapi_valueset_add_attr_valuearray_ext (a, vs_new, valstoreplace, vals_count, flags, &dupindex);

 	if ( rc == LDAP_SUCCESS )

 	{

@@ -1408,8 +1410,11 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *

 	{

 	        /* caller expects us to own valstoreplace - since we cannot

 	           use them, just delete them */

+		/* using PASSIN, some of the Slapi_Value* are in vs_new, and the rest

+		 * after dupindex are in valstoreplace

+		 */

         	slapi_valueset_free(vs_new);

-        	valuearray_free(&valstoreplace);

+        	valuearray_free_ext(&valstoreplace, dupindex);

 		PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));

 	}

     }

-- 

1.7.1