Hello,

fixed fix is attached. Clang found bug in the fix but I didn't notice that because of other warnings ...

On 11/21/2012 04:04 PM, Petr Spacek wrote:
Hello,

I noticed this problem during investigation of dead code found by Clang.

     Fix error handling in ldap_entry_create().

     Jump to cleanup section after first memory allocation created memory leak
     which crashed BIND on reload.

     Missing return value check after ldap_get_dn() call can lead to crash.

--
Petr^2 Spacek
From 2e9a53618bdeb4f450a19febdd43a0f3a0553d6b Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 21 Nov 2012 15:53:28 +0100
Subject: [PATCH] Fix error handling in ldap_entry_create().

Jump to cleanup section after first memory allocation created memory leak
which crashed BIND on reload.

Missing return value check after ldap_get_dn() call can lead to crash.

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_entry.c  | 26 ++++++++++++++++----------
 src/ldap_helper.c |  8 +++++---
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/ldap_entry.c b/src/ldap_entry.c
index 9436b895913b2eb1a711d9343e43e695ea7e6ae4..7d8b29bd0f1206df6b4fcdcdb08c4c9e82630527 100644
--- a/src/ldap_entry.c
+++ b/src/ldap_entry.c
@@ -183,9 +183,9 @@ ldap_entry_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry,
                   ldap_entry_t **entryp)
 {
 	isc_result_t result;
-	ldap_attribute_t *attr;
+	ldap_attribute_t *attr = NULL;
 	char *attribute;
-	BerElement *ber;
+	BerElement *ber = NULL;
 	ldap_entry_t *entry = NULL;
 
 	REQUIRE(ld != NULL);
@@ -213,19 +213,25 @@ ldap_entry_create(isc_mem_t *mctx, LDAP *ld, LDAPMessage *ldap_entry,
 
 		APPEND(entry->attrs, attr, link);
 	}
+	attr = NULL;
 
 	entry->dn = ldap_get_dn(ld, ldap_entry);
+	if (entry->dn == NULL) {
+		log_ldap_error(ld);
+		CLEANUP_WITH(ISC_R_FAILURE);
+	}
 
+	*entryp = entry;
+
+cleanup:
 	if (ber != NULL)
 		ber_free(ber, 0);
-
-	*entryp = entry;
-
-	return ISC_R_SUCCESS;
-
-cleanup:
-	if (entry != NULL)
-		ldap_attributelist_destroy(mctx, &entry->attrs);
+	if (result != ISC_R_SUCCESS) {
+		if (entry != NULL)
+			ldap_attributelist_destroy(mctx, &entry->attrs);
+		SAFE_MEM_PUT_PTR(mctx, entry);
+		SAFE_MEM_PUT_PTR(mctx, attr);
+	}
 
 	return result;
 }
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index c85fd7feca348bdd63bba33409fc740b399e61dd..8a7765d5a650279db0417f6fb5be01a3dd2b82d4 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -635,9 +635,9 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 		 * We use SIGUSR1 to not to interfere with any signal
 		 * used by BIND itself.
 		 */
-		REQUIRE(pthread_kill(ldap_inst->watcher, SIGUSR1) == 0);
-		RUNTIME_CHECK(isc_thread_join(ldap_inst->watcher, NULL)
-			      == ISC_R_SUCCESS);
+		//REQUIRE(pthread_kill(ldap_inst->watcher, SIGUSR1) == 0);
+		//RUNTIME_CHECK(isc_thread_join(ldap_inst->watcher, NULL)
+		//	      == ISC_R_SUCCESS);
 		ldap_inst->watcher = 0;
 	}
 
@@ -1434,6 +1434,8 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst, isc_boolean_t delete_only)
 
 	REQUIRE(ldap_inst != NULL);
 
+	//return ISC_R_FAILURE;
+
 	if (ldap_inst->psearch && !delete_only) {
 		/* Watcher does the work for us, but deletion is allowed. */
 		return ISC_R_SUCCESS;
-- 
1.7.11.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to