[Freeipa-devel] [PATCH 0076] Fix crashes on BIND reload caused by improper error handling during new zone addition

[Adam Tkac]

On Fri, Oct 05, 2012 at 05:00:14PM +0200, Petr Spacek wrote:
> Hello,
> 
>     Fix crashes on BIND reload caused by improper error handling
>     during new zone addition.
> 
>     Crash can be triggered by invalid query/transfer/update ACLs
>     or potentially by error in zr_get_zone_ptr().
> 
> I found this problem during PATCH 75 testing, so there is a new ticket:
> https://fedorahosted.org/bind-dyndb-ldap/ticket/93

Ack

> From d0e958cac75035b212f87f00fade080b025d0a23 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Fri, 5 Oct 2012 14:41:57 +0200
> Subject: [PATCH] Fix crashes on BIND reload caused by improper error handling
>  during new zone addition.
> 
> Crash can be triggered by invalid query/transfer/update ACLs
> or potentially by error in zr_get_zone_ptr().
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/93
> 
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
>  src/ldap_helper.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index d4bb6db10b0e79f8777fde3c5f344298af87ce56..0e1cf6f7a6986db126aaa5329dbe9abbc98c8bf4 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -1031,6 +1031,7 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  	isc_result_t result;
>  	isc_boolean_t unlock = ISC_FALSE;
>  	isc_boolean_t publish = ISC_FALSE;
> +	isc_boolean_t published = ISC_FALSE;
>  	isc_task_t *task = inst->task;
>  	isc_uint32_t ldap_serial;
>  	isc_uint32_t zr_serial;	/* SOA serial value from in-memory zone register */
> @@ -1074,12 +1075,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  
>  	/* Check if we are already serving given zone */
>  	result = zr_get_zone_ptr(inst->zone_register, &name, &zone);
> -	if (result != ISC_R_SUCCESS) { /* TODO: What about other errors? */
> +	if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
>  		CHECK(create_zone(inst, &name, &zone));
>  		CHECK(zr_add_zone(inst->zone_register, zone, dn));
>  		publish = ISC_TRUE;
>  		log_debug(2, "created zone %p: %s", zone, dn);
> -	}
> +	} else if (result != ISC_R_SUCCESS)
> +		CLEANUP_WITH(result);
>  
>  	log_debug(2, "Setting SSU table for %p: %s", zone, dn);
>  	/* Get the update policy and update the zone with it. */
> @@ -1119,6 +1121,7 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  	if (publish) {
>  		/* Everything is set correctly, publish zone */
>  		CHECK(publish_zone(inst, zone));
> +		published = ISC_TRUE;
>  	}
>  
>  	/*
> @@ -1178,6 +1181,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>  	}
>  
>  cleanup:
> +	if (publish && !published) { /* Failure in ACL parsing or so. */
> +		log_error_r("zone '%s': publishing failed, rolling back due to",
> +			    entry->dn);
> +		result = zr_del_zone(inst->zone_register, &name);
> +		if (result != ISC_R_SUCCESS)
> +			log_error_r("zone '%s': rollback failed", entry->dn);
> +	}
>  	if (unlock)
>  		isc_task_endexclusive(task);
>  	if (dns_name_dynamic(&name))
> -- 
> 1.7.11.4
> 


-- 
Adam Tkac, Red Hat, Inc.