[Freeipa-devel] [PATCH 101] Fix error handling in ldap_entry_create()

[Adam Tkac]

On Thu, Nov 22, 2012 at 09:56:12AM +0100, Petr Spacek wrote:
> Hello,
> 
> apparently I was very tired yesterday ... Cleaned version of the
> patch is attached.
> 
> Petr^2 Spacek
> 
> On 11/21/2012 05:04 PM, Petr Spacek wrote:
> >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.

Ack

> From eddb9db446610e79e4852b15cb2be420090364b7 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at 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 <pspacek at redhat.com>
> ---
>  src/ldap_entry.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 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;
>  }
> -- 
> 1.7.11.7
> 

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


-- 
Adam Tkac, Red Hat, Inc.