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

Reply via email to