Re: [Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code

2012-09-24 Thread Adam Tkac 
On Thu, Sep 13, 2012 at 01:37:37PM +0200, Petr Spacek wrote:
 Hello,
 
 Add missing DNS view attach/detach to LDAP instance management code.
 This fixes race condition in BIND shutdown after SIGINT:
 - failing assert caused by use-after-free in dns_zt_find():
 (((zt) != ((void *)0))  (((const isc__magic_t *)(zt))-magic
 == ((('Z')  24 | ('T')  16 | ('b')  8 | ('l')

Ack.

 From cc612198a0b7d662557a7c4f71732135e8f43025 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Thu, 13 Sep 2012 13:08:36 +0200
 Subject: [PATCH] Add missing DNS view attach/detach to LDAP instance
  management code. This fixes race condition in BIND shutdown
  after SIGINT: - failing assert caused by use-after-free in
  dns_zt_find(): (((zt) != ((void *)0))  (((const
  isc__magic_t *)(zt))-magic == ((('Z')  24 | ('T')  16
  | ('b')  8 | ('l')
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 3b49de809738fef18cae10c38fd3d9c33eef5141..658b960f50b461fa602edf51e955f3bdd4769e1d
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -333,6 +333,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
   unsigned int i;
   isc_result_t result;
   ldap_instance_t *ldap_inst;
 + dns_view_t *view = NULL;
   ld_string_t *auth_method_str = NULL;
   setting_t ldap_settings[] = {
   { uri, no_default_string  },
 @@ -369,10 +370,9 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
  
   isc_mem_attach(mctx, ldap_inst-mctx);
   ldap_inst-db_name = db_name;
 - ldap_inst-view = dns_dyndb_get_view(dyndb_args);
 + view = dns_dyndb_get_view(dyndb_args);
 + dns_view_attach(view, ldap_inst-view);
   ldap_inst-zmgr = dns_dyndb_get_zonemgr(dyndb_args);
 - /* commented out for now, cause named to hang */
 - //dns_view_attach(view, ldap_inst-view);
  
   CHECK(zr_create(mctx, ldap_inst-zone_register));
  
 @@ -616,8 +616,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
   str_destroy(ldap_inst-fake_mname);
   str_destroy(ldap_inst-ldap_hostname);
  
 - /* commented out for now, causes named to hang */
 - //dns_view_detach(ldap_inst-view);
 + dns_view_detach(ldap_inst-view);
  
   DESTROYLOCK(ldap_inst-kinit_lock);
  
 -- 
 1.7.11.4
 


-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code

2012-09-24 Thread Petr Spacek 

On 09/24/2012 04:20 PM, Adam Tkac wrote:

On Thu, Sep 13, 2012 at 01:37:37PM +0200, Petr Spacek wrote:

Hello,

 Add missing DNS view attach/detach to LDAP instance management code.
 This fixes race condition in BIND shutdown after SIGINT:
 - failing assert caused by use-after-free in dns_zt_find():
 (((zt) != ((void *)0))  (((const isc__magic_t *)(zt))-magic
 == ((('Z')  24 | ('T')  16 | ('b')  8 | ('l')


Ack.


Pushed to master:
d26d61c4add5593ffca4380029748c2d359140c5

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code

2012-09-21 Thread Petr Spacek 

On 09/20/2012 01:17 PM, Petr Spacek wrote:

On 09/13/2012 01:37 PM, Petr Spacek wrote:

Hello,

 Add missing DNS view attach/detach to LDAP instance management code.
 This fixes race condition in BIND shutdown after SIGINT:
 - failing assert caused by use-after-free in dns_zt_find():
 (((zt) != ((void *)0))  (((const isc__magic_t *)(zt))-magic
 == ((('Z')  24 | ('T')  16 | ('b')  8 | ('l')

Petr^2 Spacek


This patch causes deadlock in shut-down sequence in certain situations, I'm
still searching for the root cause.

Old comment from 2009-02-10
/* commented out for now, cause named to hang */
was correct, apparently ...

Apparently my statement above was wrong. Deadlock I observed was caused by 
incorrect zone attach/detach sequence. This problem is fixed in

[PATCH 0067] Fix error handling in ldap_get_zone_serial().

You can proceed with reviewing this patch.

Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code

2012-09-20 Thread Petr Spacek 

On 09/13/2012 01:37 PM, Petr Spacek wrote:

Hello,

 Add missing DNS view attach/detach to LDAP instance management code.
 This fixes race condition in BIND shutdown after SIGINT:
 - failing assert caused by use-after-free in dns_zt_find():
 (((zt) != ((void *)0))  (((const isc__magic_t *)(zt))-magic
 == ((('Z')  24 | ('T')  16 | ('b')  8 | ('l')

Petr^2 Spacek


This patch causes deadlock in shut-down sequence in certain situations, I'm 
still searching for the root cause.


Old comment from 2009-02-10
/* commented out for now, cause named to hang */
was correct, apparently ...

--
Petr^2 Spacek

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

[Freeipa-devel] [PATCH 0061] Add missing DNS view attach/detach to LDAP instance management code

2012-09-13 Thread Petr Spacek 

Hello,

Add missing DNS view attach/detach to LDAP instance management code.
This fixes race condition in BIND shutdown after SIGINT:
- failing assert caused by use-after-free in dns_zt_find():
(((zt) != ((void *)0))  (((const isc__magic_t *)(zt))-magic
== ((('Z')  24 | ('T')  16 | ('b')  8 | ('l')

Petr^2 Spacek
From cc612198a0b7d662557a7c4f71732135e8f43025 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 13 Sep 2012 13:08:36 +0200
Subject: [PATCH] Add missing DNS view attach/detach to LDAP instance
 management code. This fixes race condition in BIND shutdown
 after SIGINT: - failing assert caused by use-after-free in
 dns_zt_find(): (((zt) != ((void *)0))  (((const
 isc__magic_t *)(zt))-magic == ((('Z')  24 | ('T')  16
 | ('b')  8 | ('l')

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 3b49de809738fef18cae10c38fd3d9c33eef5141..658b960f50b461fa602edf51e955f3bdd4769e1d 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -333,6 +333,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
 	unsigned int i;
 	isc_result_t result;
 	ldap_instance_t *ldap_inst;
+	dns_view_t *view = NULL;
 	ld_string_t *auth_method_str = NULL;
 	setting_t ldap_settings[] = {
 		{ uri,	 no_default_string		},
@@ -369,10 +370,9 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
 
 	isc_mem_attach(mctx, ldap_inst-mctx);
 	ldap_inst-db_name = db_name;
-	ldap_inst-view = dns_dyndb_get_view(dyndb_args);
+	view = dns_dyndb_get_view(dyndb_args);
+	dns_view_attach(view, ldap_inst-view);
 	ldap_inst-zmgr = dns_dyndb_get_zonemgr(dyndb_args);
-	/* commented out for now, cause named to hang */
-	//dns_view_attach(view, ldap_inst-view);
 
 	CHECK(zr_create(mctx, ldap_inst-zone_register));
 
@@ -616,8 +616,7 @@ destroy_ldap_instance(ldap_instance_t **ldap_instp)
 	str_destroy(ldap_inst-fake_mname);
 	str_destroy(ldap_inst-ldap_hostname);
 
-	/* commented out for now, causes named to hang */
-	//dns_view_detach(ldap_inst-view);
+	dns_view_detach(ldap_inst-view);
 
 	DESTROYLOCK(ldap_inst-kinit_lock);
 
-- 
1.7.11.4

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