On Fri, Sep 07, 2012 at 01:05:37PM +0200, Petr Spacek wrote: > Hello, > > Fix race condition in addrdataset() during SOA serial update. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/89
Good catch, thanks. Ack A > From 5e8bc8f943345d8d92900474905288939958dcd8 Mon Sep 17 00:00:00 2001 > From: Petr Spacek <pspa...@redhat.com> > Date: Fri, 7 Sep 2012 13:01:57 +0200 > Subject: [PATCH] Fix race condition in addrdataset() during SOA serial > update. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/89 > > Signed-off-by: Petr Spacek <pspa...@redhat.com> > --- > src/ldap_driver.c | 44 ++++++++++++++++++++++++++++++++++---------- > src/ldap_helper.c | 4 ++-- > 2 files changed, 36 insertions(+), 12 deletions(-) > > diff --git a/src/ldap_driver.c b/src/ldap_driver.c > index > 2cdde30cdad9544d530475f5cf4a0b8275a56f03..3a802238028145d35390f6a8d00f156bfdf8e7a1 > 100644 > --- a/src/ldap_driver.c > +++ b/src/ldap_driver.c > @@ -936,6 +936,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, > dns_dbversion_t *version, > dns_rdatalist_t diff; > isc_result_t result; > isc_boolean_t rdatalist_exists = ISC_FALSE; > + isc_boolean_t soa_simulated_write = ISC_FALSE; > > UNUSED(now); > UNUSED(db); > @@ -975,42 +976,65 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, > dns_dbversion_t *version, > rdatalist_removedups(found_rdlist, new_rdlist, > ISC_FALSE, &diff); > > - if ((options & DNS_DBADD_MERGE) != 0) > + if ((options & DNS_DBADD_MERGE) == 0 && > + (rdatalist_length(&diff) != 0)) { > + CLEANUP_WITH(DNS_R_NOTEXACT); > + } else { > free_rdatalist(ldapdb->common.mctx, &diff); > - else if (rdatalist_length(&diff) != 0) { > - free_rdatalist(ldapdb->common.mctx, &diff); > - result = DNS_R_NOTEXACT; > - goto cleanup; > } > } else { > /* Replace existing rdataset */ > free_rdatalist(ldapdb->common.mctx, found_rdlist); > } > } > > - CHECK(write_to_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, new_rdlist)); > + /* HACK: SOA addition will never fail with DNS_R_UNCHANGED. > + * This prevents warning from BIND's diff_apply(), it has too strict > + * checks for us. > + * > + * Reason: There is a race condition between SOA serial update > + * from BIND's update_action() and our persistent search watcher, > because > + * they don't know about each other. > + * BIND's update_action() changes data with first addrdataset() call and > + * then changes serial with second addrdataset() call. > + * It can lead to empty diff if persistent search watcher > + * incremented serial in meanwhile. > + */ > + if (HEAD(new_rdlist->rdata) == NULL) { > + if (rdlist->type == dns_rdatatype_soa) > + soa_simulated_write = ISC_TRUE; > + else > + CLEANUP_WITH(DNS_R_UNCHANGED); > + } else { > + CHECK(write_to_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, > new_rdlist)); > + } > + > > if (addedrdataset != NULL) { > - result = dns_rdatalist_tordataset(new_rdlist, addedrdataset); > - /* Use strong condition here, returns only SUCCESS */ > - INSIST(result == ISC_R_SUCCESS); > + if (soa_simulated_write) { > + dns_rdataset_clone(rdataset, addedrdataset); > + } else { > + result = dns_rdatalist_tordataset(new_rdlist, > addedrdataset); > + /* Use strong condition here, returns only SUCCESS */ > + INSIST(result == ISC_R_SUCCESS); > + } > } > > if (rdatalist_exists) { > ISC_LIST_APPENDLIST(found_rdlist->rdata, new_rdlist->rdata, > link); > SAFE_MEM_PUT_PTR(ldapdb->common.mctx, new_rdlist); > } else > APPEND(ldapdbnode->rdatalist, new_rdlist, link); > > - > return ISC_R_SUCCESS; > > cleanup: > if (new_rdlist != NULL) { > free_rdatalist(ldapdb->common.mctx, new_rdlist); > SAFE_MEM_PUT_PTR(ldapdb->common.mctx, new_rdlist); > } > + free_rdatalist(ldapdb->common.mctx, &diff); > > return result; > } > diff --git a/src/ldap_helper.c b/src/ldap_helper.c > index > 3241ffe486205fa03a6fd1a0a14edf1245c5c4aa..e636a84b35d0bcdc8573c6e7146f38ee21a42076 > 100644 > --- a/src/ldap_helper.c > +++ b/src/ldap_helper.c > @@ -2973,10 +2973,10 @@ soa_serial_increment(isc_mem_t *mctx, ldap_instance_t > *inst, > > /* put the new SOA to inst->cache and compare old and new serials */ > CHECK(ldap_get_zone_serial(inst, zone_name, &new_serial)); > - INSIST(isc_serial_gt(new_serial, old_serial) == ISC_TRUE); > > cleanup: > - if (result != ISC_R_SUCCESS) > + if (result != ISC_R_SUCCESS || > + isc_serial_gt(new_serial, old_serial) != ISC_TRUE) > log_error("SOA serial number incrementation failed in zone > '%s'", > str_buf(zone_dn)); > > -- > 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
