Re: [Freeipa-devel] [PATCH 0031] Prevent crashes in ldap_pool_*() function family

2012-07-13 Thread Petr Spacek

On 07/13/2012 03:48 PM, Adam Tkac wrote:

On Thu, Jul 12, 2012 at 05:18:35PM +0200, Petr Spacek wrote:

Hello,

this patch fixes occasional crashes caused by incorrect error
handling in ldap_pool_*() functions.

https://fedorahosted.org/bind-dyndb-ldap/ticket/84

It can be caused by memory allocation error OR timeout during
connection establishing phase.

To trigger this problem first connection has to be established
properly and some other connection has to fail. It is not enough to
timeout at first connection/try, that case was handled properly.


Ack, please push it to master.

Regards, Adam


Pushed to master: 
https://fedorahosted.org/bind-dyndb-ldap/changeset/e44ce4d9c42ad9b1226cea5b62e9040f2d7e4df2


Thanks for your time!

Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0031] Prevent crashes in ldap_pool_*() function family

2012-07-13 Thread Adam Tkac
On Thu, Jul 12, 2012 at 05:18:35PM +0200, Petr Spacek wrote:
> Hello,
> 
> this patch fixes occasional crashes caused by incorrect error
> handling in ldap_pool_*() functions.
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/84
> 
> It can be caused by memory allocation error OR timeout during
> connection establishing phase.
> 
> To trigger this problem first connection has to be established
> properly and some other connection has to fail. It is not enough to
> timeout at first connection/try, that case was handled properly.

Ack, please push it to master.

Regards, Adam

> From 7ef5c14ffa69cc4d60a76c9db63b8e3ce065d27b Mon Sep 17 00:00:00 2001
> From: Petr Spacek 
> Date: Thu, 12 Jul 2012 17:10:58 +0200
> Subject: [PATCH] Prevent crashes in ldap_pool_*() function family.
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/84
> 
> Signed-off-by: Petr Spacek 
> ---
>  src/ldap_helper.c |   18 +++---
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 
> aa7f97664d3fd6de43b0ee7b7e6caa0fc0e25dde..dc18d8d51c4980448fa78ae1604c78782d601113
>  100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -538,6 +538,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
>  
>   dns_rbtnodechain_invalidate(&chain);
>  
> + /* TODO: Terminate psearch watcher sooner? */
>   if (ldap_inst->psearch && ldap_inst->watcher != 0) {
>   ldap_inst->exiting = ISC_TRUE;
>   /*
> @@ -628,9 +629,12 @@ destroy_ldap_connection(ldap_pool_t *pool, 
> ldap_connection_t **ldap_connp)
>  {
>   ldap_connection_t *ldap_conn;
>  
> - REQUIRE(ldap_connp != NULL && *ldap_connp != NULL);
> + REQUIRE(ldap_connp != NULL);
>  
>   ldap_conn = *ldap_connp;
> + if (ldap_conn == NULL)
> + return;
> +
>   DESTROYLOCK(&ldap_conn->lock);
>   if (ldap_conn->handle != NULL)
>   ldap_unbind_ext_s(ldap_conn->handle, NULL, NULL);
> @@ -2603,20 +2607,21 @@ ldap_pool_create(isc_mem_t *mctx, unsigned int 
> connections, ldap_pool_t **poolp)
>   return ISC_R_SUCCESS;
>  
>  cleanup:
> - if (pool != NULL)
> - ldap_pool_destroy(&pool);
> + ldap_pool_destroy(&pool);
>   return result;
>  }
>  static void
>  ldap_pool_destroy(ldap_pool_t **poolp)
>  {
>   ldap_pool_t *pool;
>   ldap_connection_t *ldap_conn;
>   unsigned int i;
>  
> - REQUIRE(poolp != NULL && *poolp != NULL);
> + REQUIRE(poolp != NULL);
>  
>   pool = *poolp;
> + if (pool == NULL)
> + return;
>  
>   for (i = 0; i < pool->connections; i++) {
>   ldap_conn = pool->conns[i];
> @@ -2630,6 +2635,7 @@ ldap_pool_destroy(ldap_pool_t **poolp)
>   semaphore_destroy(&pool->conn_semaphore);
>  
>   MEM_PUT_AND_DETACH(pool);
> + *poolp = NULL;
>  }
>  
>  static isc_result_t
> @@ -2701,9 +2707,7 @@ ldap_pool_connect(ldap_pool_t *pool, ldap_instance_t 
> *ldap_inst)
>  
>  cleanup:
>   for (i = 0; i < pool->connections; i++) {
> - ldap_conn = pool->conns[i];
> - if (ldap_conn != NULL)
> - destroy_ldap_connection(pool, &ldap_conn);
> + destroy_ldap_connection(pool, &pool->conns[i]);
>   }
>   return result;
>  }
> -- 
> 1.7.7.6
> 


-- 
Adam Tkac, Red Hat, Inc.

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