[Freeipa-devel] [PATCH 0038] Fix two memory leaks in ldap_query()

[Adam Tkac]

On Fri, Jul 20, 2012 at 02:28:09PM +0200, Petr Spacek wrote:
> Hello,
> 
> this patch fixes two memory leaks in ldap_query(). Both memory leaks
> occurs after "non-success" queries.
> 
> It effectively re-implements fix for "ldap_query can incorrectly
> return ISC_R_SUCCESS even when failed":
> http://git.fedorahosted.org/git/?p=bind-dyndb-ldap.git;a=commitdiff;h=a7cd8ae747b3a81a02ab9e5dbefe1c595aa24ff6
> 
> Please double-check this approach.

Ack, please push it to master.

A

> From c8718b98641e7537b2350a625b03b0b7fec6f206 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Fri, 20 Jul 2012 14:18:41 +0200
> Subject: [PATCH] Fix two memory leaks in ldap_query().
> 
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
>  src/ldap_helper.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 6ac76faecbc250deb28203256fa13d83ae6c80f4..daffac7c7825a99a07c333217638d3beaddfaad2 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -1753,28 +1753,30 @@ retry:
>  					       &ldap_qresult->ldap_entries);
>  		if (result != ISC_R_SUCCESS) {
>  			log_error("failed to save LDAP query results");
> -			return result;
> +			goto cleanup;
>  		}
>  
>  		*ldap_qresultp = ldap_qresult;
>  		return ISC_R_SUCCESS;
> +	} else {
> +		result = ISC_R_FAILURE;
>  	}
>  
>  	ret = ldap_get_option(ldap_conn->handle, LDAP_OPT_RESULT_CODE,
>  			      (void *)&ldap_err_code);
> -	if (ret == LDAP_OPT_SUCCESS && ldap_err_code == LDAP_NO_SUCH_OBJECT)
> -		return ISC_R_NOTFOUND;
> -	/* some error happened during ldap_search, try to recover */
> -	else if (!once) {
> +	if (ret == LDAP_OPT_SUCCESS && ldap_err_code == LDAP_NO_SUCH_OBJECT) {
> +		result = ISC_R_NOTFOUND;
> +	} else if (!once) {
> +		/* some error happened during ldap_search, try to recover */
>  		once++;
>  		result = handle_connection_error(ldap_inst, ldap_conn,
>  						 ISC_FALSE);
>  		if (result == ISC_R_SUCCESS)
>  			goto retry;
>  	}
>  cleanup:
>  	ldap_query_free(ISC_FALSE, &ldap_qresult);
> -	return ISC_R_FAILURE;
> +	return result;
>  }
>  
>  /**
> -- 
> 1.7.7.6
> 


-- 
Adam Tkac, Red Hat, Inc.