Blame SOURCES/0004-Ticket-47455-valgrind-value-mem-leaks-uninit-mem-usa.patch

ba46c7
From 6954ce25351d8dd2c88d57b51d3b91900e3de605 Mon Sep 17 00:00:00 2001
ba46c7
From: Rich Megginson <rmeggins@redhat.com>
ba46c7
Date: Thu, 5 Sep 2013 19:45:44 -0600
ba46c7
Subject: [PATCH 4/4] Ticket #47455 - valgrind - value mem leaks, uninit mem usage
ba46c7
ba46c7
https://fedorahosted.org/389/ticket/47455
ba46c7
Reviewed by: nkinder (Thanks!)
ba46c7
Branch: 389-ds-base-1.3.1
ba46c7
Fix Description: The problem was that slapi_valueset_add_attr_valuearray_ext
ba46c7
was assuming the caller was going to free the entire given vs upon failure.
ba46c7
This is fine for the value replace case but not for the add 1 value case.
ba46c7
Callers of slapi_valueset_add_attr_valuearray_ext must provide
ba46c7
the dup_index parameter if using SLAPI_VALUE_FLAG_PASSIN and
ba46c7
SLAPI_VALUE_FLAG_DUPCHECK, and if there is more than one value.  The caller
ba46c7
needs to know which of the values from addvals is in vs to properly clean up
ba46c7
with no memory leaks.
ba46c7
Platforms tested: RHEL6 x86_64
ba46c7
Flag Day: no
ba46c7
Doc impact: no
ba46c7
(cherry picked from commit 3adc242bcc8c6d0d05d5d9773f32b4f81afb6e6d)
ba46c7
(cherry picked from commit 6357ced2e4380def053966e849eac45e44009662)
ba46c7
---
ba46c7
 ldap/servers/slapd/slapi-private.h |    5 +++++
ba46c7
 ldap/servers/slapd/valueset.c      |   13 +++++++++----
ba46c7
 2 files changed, 14 insertions(+), 4 deletions(-)
ba46c7
ba46c7
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
ba46c7
index b2364ab..194f3fd 100644
ba46c7
--- a/ldap/servers/slapd/slapi-private.h
ba46c7
+++ b/ldap/servers/slapd/slapi-private.h
ba46c7
@@ -846,6 +846,11 @@ void valuearray_add_valuearray_fast( Slapi_Value ***vals, Slapi_Value **addvals,
ba46c7
 Slapi_Value * valueset_find_sorted (const Slapi_Attr *a, const Slapi_ValueSet *vs, const Slapi_Value *v, int *index);
ba46c7
 int valueset_insert_value_to_sorted(const Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *vi, int dupcheck);
ba46c7
 void valueset_array_to_sorted (const Slapi_Attr *a, Slapi_ValueSet *vs);
ba46c7
+/* NOTE: if the flags include SLAPI_VALUE_FLAG_PASSIN and SLAPI_VALUE_FLAG_DUPCHECK
ba46c7
+ * THE CALLER MUST PROVIDE THE dup_index PARAMETER in order to know where in addval
ba46c7
+ * the un-copied values start e.g. to free them for cleanup
ba46c7
+ * see valueset_replace_valuearray_ext() for an example
ba46c7
+ */
ba46c7
 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);
ba46c7
 int valuearray_find(const Slapi_Attr *a, Slapi_Value **va, const Slapi_Value *v);
ba46c7
 int valuearray_dn_normalize_value(Slapi_Value **vals);
ba46c7
diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c
ba46c7
index 85d2274..8a4bd03 100644
ba46c7
--- a/ldap/servers/slapd/valueset.c
ba46c7
+++ b/ldap/servers/slapd/valueset.c
ba46c7
@@ -1134,8 +1134,9 @@ slapi_valueset_add_attr_valuearray_ext(const Slapi_Attr *a, Slapi_ValueSet *vs,
ba46c7
 					rc = LDAP_TYPE_OR_VALUE_EXISTS;
ba46c7
 					if (dup_index) *dup_index = i;
ba46c7
 					if (passin) {
ba46c7
-						/* we have to NULL out the first value so valuearray_free won't delete values in addvals */
ba46c7
-						(vs->va)[0] = NULL;
ba46c7
+						PR_ASSERT((i == 0) || dup_index);
ba46c7
+						/* caller must provide dup_index to know how far we got in addvals */
ba46c7
+						(vs->va)[vs->num] = NULL;
ba46c7
 					} else {
ba46c7
 						slapi_value_free(&(vs->va)[vs->num]);
ba46c7
 					}
ba46c7
@@ -1379,8 +1380,9 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *
ba46c7
     } else {
ba46c7
 	/* verify the given values are not duplicated.  */
ba46c7
 	unsigned long flags = SLAPI_VALUE_FLAG_PASSIN|SLAPI_VALUE_FLAG_DUPCHECK;
ba46c7
+	int dupindex = 0;
ba46c7
 	Slapi_ValueSet *vs_new = slapi_valueset_new();
ba46c7
-	rc = slapi_valueset_add_attr_valuearray_ext (a, vs_new, valstoreplace, vals_count, flags, NULL);
ba46c7
+	rc = slapi_valueset_add_attr_valuearray_ext (a, vs_new, valstoreplace, vals_count, flags, &dupindex);
ba46c7
 
ba46c7
 	if ( rc == LDAP_SUCCESS )
ba46c7
 	{
ba46c7
@@ -1408,8 +1410,11 @@ valueset_replace_valuearray_ext(Slapi_Attr *a, Slapi_ValueSet *vs, Slapi_Value *
ba46c7
 	{
ba46c7
 	        /* caller expects us to own valstoreplace - since we cannot
ba46c7
 	           use them, just delete them */
ba46c7
+		/* using PASSIN, some of the Slapi_Value* are in vs_new, and the rest
ba46c7
+		 * after dupindex are in valstoreplace
ba46c7
+		 */
ba46c7
         	slapi_valueset_free(vs_new);
ba46c7
-        	valuearray_free(&valstoreplace);
ba46c7
+        	valuearray_free_ext(&valstoreplace, dupindex);
ba46c7
 		PR_ASSERT((vs->sorted == NULL) || (vs->num == 0) || ((vs->sorted[0] >= 0) && (vs->sorted[0] < vs->num)));
ba46c7
 	}
ba46c7
     }
ba46c7
-- 
ba46c7
1.7.1
ba46c7