On Mon, Oct 08, 2012 at 04:46:54PM +0200, Petr Spacek wrote: > Hello, > > Use automatic connection management in LDAP modification code to > prevent potential deadlock. > > Without this patch the plugin will deadlock when modify_ldap_common() > is called with PTR synchronization enabled and only single > connection is available in the connection pool.
Nack If I read the patch correctly, it leaves unused ldap_conn parameters in ldap_modify_do() and modify_soa_record() functions. Those params are always NULL so they can be safely removed. Please also remove the "autoconn" variable from ldap_modify_do() Regards, Adam > From 5ad686a95510b1584c85d672ec845b27504f746c Mon Sep 17 00:00:00 2001 > From: Petr Spacek <[email protected]> > Date: Mon, 8 Oct 2012 16:41:40 +0200 > Subject: [PATCH] Use automatic connection management in LDAP modification > code to prevent potential deadlock. > > Without this patch the plugin will deadlock when modify_ldap_common() > is called with PTR synchronization enabled and only single > connection is available in the connection pool. > > Signed-off-by: Petr Spacek <[email protected]> > --- > src/ldap_helper.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/src/ldap_helper.c b/src/ldap_helper.c > index > f8df1b29871c28a1eeecfa93d5d91edd1aee3944..03adb83bdc34593ec527facd3e3fc60755a4de22 > 100644 > --- a/src/ldap_helper.c > +++ b/src/ldap_helper.c > @@ -2593,7 +2593,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t > *ldap_inst, > { > isc_result_t result; > isc_mem_t *mctx = ldap_inst->mctx; > - ldap_connection_t *ldap_conn = NULL; > ld_string_t *owner_dn = NULL; > LDAPMod *change[3] = { NULL }; > LDAPMod *change_ptr = NULL; > @@ -2630,8 +2629,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t > *ldap_inst, > > CHECK(dn_to_dnsname(mctx, zone_dn, &zone_name, NULL)); > > - CHECK(ldap_pool_getconnection(ldap_inst->pool, &ldap_conn)); > - > result = zr_get_zone_settings(ldap_inst->zone_register, &zone_name, > &zone_settings); > if (result != ISC_R_SUCCESS) { > @@ -2655,7 +2652,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t > *ldap_inst, > CHECK(discard_from_cache(cache, owner)); > > if (rdlist->type == dns_rdatatype_soa) { > - result = modify_soa_record(ldap_inst, ldap_conn, > str_buf(owner_dn), > + result = modify_soa_record(ldap_inst, NULL, str_buf(owner_dn), > HEAD(rdlist->rdata)); > goto cleanup; > } > @@ -2666,7 +2663,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t > *ldap_inst, > CHECK(ldap_rdttl_to_ldapmod(mctx, rdlist, &change[1])); > } > > - CHECK(ldap_modify_do(ldap_inst, ldap_conn, str_buf(owner_dn), change, > delete_node)); > + CHECK(ldap_modify_do(ldap_inst, NULL, str_buf(owner_dn), change, > delete_node)); > > /* Keep the PTR of corresponding A/AAAA record synchronized. */ > if (rdlist->type == dns_rdatatype_a || rdlist->type == > dns_rdatatype_aaaa) { > @@ -2824,13 +2821,13 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t > *ldap_inst, > change_ptr = NULL; > > /* Modify PTR record. */ > - CHECK(ldap_modify_do(ldap_inst, ldap_conn, > str_buf(owner_dn_ptr), change, delete_node)); > + CHECK(ldap_modify_do(ldap_inst, NULL, str_buf(owner_dn_ptr), > + change, delete_node)); > (void) discard_from_cache(ldap_instance_getcache(ldap_inst), > dns_fixedname_name(&ptr_name)); > } > > cleanup: > - ldap_pool_putconnection(ldap_inst->pool, &ldap_conn); > str_destroy(&owner_dn_ptr); > str_destroy(&owner_dn); > str_destroy(&str_ptr); > -- > 1.7.11.4 > -- Adam Tkac, Red Hat, Inc. _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
