Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
On 17.6.2014 09:33, Tomas Hozza wrote: - Original Message - On 28.5.2014 13:26, Tomas Hozza wrote: On 05/27/2014 03:59 PM, Petr Spacek wrote: On 27.5.2014 15:54, Petr Spacek wrote: Fix race condition during zone loading. DNS zone has to be added to DNS view before dns_zone_load() is called. It is necessary to prevent dns_zone_load() from racing with dns_zone_setview(). This race condition sometimes prevents zone from being signed. Now the unsigned zone is visible until signing process is complete. This mimics BIND behavior for in-line signed zones. https://fedorahosted.org/bind-dyndb-ldap/ticket/56 And here is the patch... Hi. When I use bind-dyndb-ldap plugin with this patch, named does not start due to: rbt.c:1379: REQUIRE(name-buffer != ((void *)0)) failed, back trace (gdb) bt #0 0x7f3963924c39 in raise () from /lib64/libc.so.6 #1 0x7f3963926348 in abort () from /lib64/libc.so.6 #2 0x7f3966979aee in assertion_failed () #3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95 #4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from /lib64/libdns.so.100 #5 0x7f396011824b in rbt_iter_getnodename (iter=optimized out, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46 #6 0x7f396011839b in rbt_iter_next (iterp=iterp@entry=0x7f39625f8b90, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144 #7 0x7f3960112d32 in activate_zones (task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164 #8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790, event=0x7f396005b010) at syncrepl.c:138 #9 0x7f3964b8b836 in run () from /lib64/libisc.so.95 #10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0 #11 0x7f39639e3ded in clone () from /lib64/libc.so.6 It looks like you should use INIT_BUFFERED_NAME(name); used in the original code instead of dns_name_init(name, NULL). The macro initializes the buffer in name, which is missing in the new code. Oh yes, it didn't happened on my machine because I have had only single zone defined in LDAP at the time of testing. Thank you for catching this! I'm attaching fixed patch. dns_name_reset() is good enough in this case. -- Petr^2 Spacek Now it works. ACK This is delayed push notice: 129e54db4fb9ccbb85f2445db81d9f0c89722887 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
- Original Message - On 28.5.2014 13:26, Tomas Hozza wrote: On 05/27/2014 03:59 PM, Petr Spacek wrote: On 27.5.2014 15:54, Petr Spacek wrote: Fix race condition during zone loading. DNS zone has to be added to DNS view before dns_zone_load() is called. It is necessary to prevent dns_zone_load() from racing with dns_zone_setview(). This race condition sometimes prevents zone from being signed. Now the unsigned zone is visible until signing process is complete. This mimics BIND behavior for in-line signed zones. https://fedorahosted.org/bind-dyndb-ldap/ticket/56 And here is the patch... Hi. When I use bind-dyndb-ldap plugin with this patch, named does not start due to: rbt.c:1379: REQUIRE(name-buffer != ((void *)0)) failed, back trace (gdb) bt #0 0x7f3963924c39 in raise () from /lib64/libc.so.6 #1 0x7f3963926348 in abort () from /lib64/libc.so.6 #2 0x7f3966979aee in assertion_failed () #3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95 #4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from /lib64/libdns.so.100 #5 0x7f396011824b in rbt_iter_getnodename (iter=optimized out, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46 #6 0x7f396011839b in rbt_iter_next (iterp=iterp@entry=0x7f39625f8b90, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144 #7 0x7f3960112d32 in activate_zones (task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164 #8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790, event=0x7f396005b010) at syncrepl.c:138 #9 0x7f3964b8b836 in run () from /lib64/libisc.so.95 #10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0 #11 0x7f39639e3ded in clone () from /lib64/libc.so.6 It looks like you should use INIT_BUFFERED_NAME(name); used in the original code instead of dns_name_init(name, NULL). The macro initializes the buffer in name, which is missing in the new code. Oh yes, it didn't happened on my machine because I have had only single zone defined in LDAP at the time of testing. Thank you for catching this! I'm attaching fixed patch. dns_name_reset() is good enough in this case. -- Petr^2 Spacek Now it works. ACK Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
On 05/27/2014 03:59 PM, Petr Spacek wrote: On 27.5.2014 15:54, Petr Spacek wrote: Fix race condition during zone loading. DNS zone has to be added to DNS view before dns_zone_load() is called. It is necessary to prevent dns_zone_load() from racing with dns_zone_setview(). This race condition sometimes prevents zone from being signed. Now the unsigned zone is visible until signing process is complete. This mimics BIND behavior for in-line signed zones. https://fedorahosted.org/bind-dyndb-ldap/ticket/56 And here is the patch... Hi. When I use bind-dyndb-ldap plugin with this patch, named does not start due to: rbt.c:1379: REQUIRE(name-buffer != ((void *)0)) failed, back trace (gdb) bt #0 0x7f3963924c39 in raise () from /lib64/libc.so.6 #1 0x7f3963926348 in abort () from /lib64/libc.so.6 #2 0x7f3966979aee in assertion_failed () #3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95 #4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from /lib64/libdns.so.100 #5 0x7f396011824b in rbt_iter_getnodename (iter=optimized out, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46 #6 0x7f396011839b in rbt_iter_next (iterp=iterp@entry=0x7f39625f8b90, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144 #7 0x7f3960112d32 in activate_zones (task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164 #8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790, event=0x7f396005b010) at syncrepl.c:138 #9 0x7f3964b8b836 in run () from /lib64/libisc.so.95 #10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0 #11 0x7f39639e3ded in clone () from /lib64/libc.so.6 It looks like you should use INIT_BUFFERED_NAME(name); used in the original code instead of dns_name_init(name, NULL). The macro initializes the buffer in name, which is missing in the new code. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
On 28.5.2014 13:26, Tomas Hozza wrote:
On 05/27/2014 03:59 PM, Petr Spacek wrote:
On 27.5.2014 15:54, Petr Spacek wrote:
Fix race condition during zone loading.
DNS zone has to be added to DNS view before dns_zone_load() is called.
It is necessary to prevent dns_zone_load() from racing with
dns_zone_setview().
This race condition sometimes prevents zone from being signed.
Now the unsigned zone is visible until signing process is complete. This
mimics BIND behavior for in-line signed zones.
https://fedorahosted.org/bind-dyndb-ldap/ticket/56
And here is the patch...
Hi.
When I use bind-dyndb-ldap plugin with this patch, named
does not start due to:
rbt.c:1379: REQUIRE(name-buffer != ((void *)0)) failed, back trace
(gdb) bt
#0 0x7f3963924c39 in raise () from /lib64/libc.so.6
#1 0x7f3963926348 in abort () from /lib64/libc.so.6
#2 0x7f3966979aee in assertion_failed ()
#3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95
#4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from
/lib64/libdns.so.100
#5 0x7f396011824b in rbt_iter_getnodename (iter=optimized out,
nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46
#6 0x7f396011839b in rbt_iter_next
(iterp=iterp@entry=0x7f39625f8b90,
nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144
#7 0x7f3960112d32 in activate_zones
(task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164
#8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790,
event=0x7f396005b010) at syncrepl.c:138
#9 0x7f3964b8b836 in run () from /lib64/libisc.so.95
#10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0
#11 0x7f39639e3ded in clone () from /lib64/libc.so.6
It looks like you should use INIT_BUFFERED_NAME(name); used in the
original code instead of dns_name_init(name, NULL). The macro
initializes the buffer in name, which is missing in the new code.
Oh yes, it didn't happened on my machine because I have had only single zone
defined in LDAP at the time of testing. Thank you for catching this!
I'm attaching fixed patch. dns_name_reset() is good enough in this case.
--
Petr^2 Spacek
From 2be7e3201f11e1322309534ab6762d637c8642c1 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 27 May 2014 15:41:59 +0200
Subject: [PATCH] Fix race condition during zone loading.
DNS zone has to be added to DNS view before dns_zone_load() is called.
It is necessary to prevent dns_zone_load() from racing with dns_zone_setview().
This race condition sometimes prevents zone from being signed.
Now the unsigned zone is visible until signing process is complete. This
mimics BIND behavior for in-line signed zones.
https://fedorahosted.org/bind-dyndb-ldap/ticket/56
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 97 ++-
src/ldap_helper.h | 2 +-
2 files changed, 54 insertions(+), 45 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 3150b56f118b6270bb79a8bc2491c472b98477dc..b285d548d9a4ccbfb008ff024d4b6281315c629f 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1100,65 +1100,74 @@ cleanup:
}
/**
+ * Add zone to view and call dns_zone_load().
+ */
+static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
+activate_zone(isc_task_t *task, ldap_instance_t *inst, dns_name_t *name) {
+ isc_result_t result;
+ dns_zone_t *raw = NULL;
+ dns_zone_t *secure = NULL;
+ dns_zone_t *toview = NULL;
+ settings_set_t *zone_settings = NULL;
+
+ CHECK(zr_get_zone_ptr(inst-zone_register, name, raw, secure));
+
+ /* Load only secure zone if inline-signing is active.
+ * It will not work if raw zone is loaded explicitly
+ * - dns_zone_load() will fail magically. */
+ toview = (secure != NULL) ? secure : raw;
+
+ /*
+ * Zone has to be published *before* zone load
+ * otherwise it will race with zone-view != NULL check
+ * in zone_maintenance() in zone.c.
+ */
+ result = publish_zone(task, inst, toview);
+ if (result != ISC_R_SUCCESS) {
+ dns_zone_log(toview, ISC_LOG_ERROR,
+ cannot add zone to view: %s,
+ dns_result_totext(result));
+ goto cleanup;
+ }
+
+ CHECK(load_zone(toview));
+ if (secure != NULL) {
+ CHECK(zr_get_zone_settings(inst-zone_register, name,
+ zone_settings));
+ CHECK(zone_master_reconfigure_nsec3param(zone_settings,
+ secure));
+ }
+
+cleanup:
+ if (raw != NULL)
+ dns_zone_detach(raw);
+ if (secure != NULL)
+ dns_zone_detach(secure);
+ return result;
+}
+
+/**
* Add all zones in zone register to DNS view specified in inst-view
* and load zones.
*/
isc_result_t
activate_zones(isc_task_t *task, ldap_instance_t *inst) {
isc_result_t result;
- isc_boolean_t loaded;
rbt_iterator_t *iter = NULL;
- dns_zone_t *raw = NULL;
- dns_zone_t *secure = NULL;
- dns_zone_t *toview = NULL;
DECLARE_BUFFERED_NAME(name);
unsigned int published_cnt = 0;
unsigned int total_cnt = 0;
- settings_set_t *zone_settings = NULL;
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
On 27.5.2014 15:54, Petr Spacek wrote:
Fix race condition during zone loading.
DNS zone has to be added to DNS view before dns_zone_load() is called.
It is necessary to prevent dns_zone_load() from racing with dns_zone_setview().
This race condition sometimes prevents zone from being signed.
Now the unsigned zone is visible until signing process is complete. This
mimics BIND behavior for in-line signed zones.
https://fedorahosted.org/bind-dyndb-ldap/ticket/56
And here is the patch...
--
Petr^2 Spacek
From 9dd8cb898c674ad3cdd0bf5fe6c08770abc04176 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 27 May 2014 15:41:59 +0200
Subject: [PATCH] Fix race condition during zone loading.
DNS zone has to be added to DNS view before dns_zone_load() is called.
It is necessary to prevent dns_zone_load() from racing with dns_zone_setview().
This race condition sometimes prevents zone from being signed.
Now the unsigned zone is visible until signing process is complete. This
mimics BIND behavior for in-line signed zones.
https://fedorahosted.org/bind-dyndb-ldap/ticket/56
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 97 ++-
src/ldap_helper.h | 2 +-
2 files changed, 54 insertions(+), 45 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 3150b56f118b6270bb79a8bc2491c472b98477dc..d01d0de1654e9ebcd9bda7aaeb930a243e24bf3c 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1100,65 +1100,74 @@ cleanup:
}
/**
+ * Add zone to view and call dns_zone_load().
+ */
+static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
+activate_zone(isc_task_t *task, ldap_instance_t *inst, dns_name_t *name) {
+ isc_result_t result;
+ dns_zone_t *raw = NULL;
+ dns_zone_t *secure = NULL;
+ dns_zone_t *toview = NULL;
+ settings_set_t *zone_settings = NULL;
+
+ CHECK(zr_get_zone_ptr(inst-zone_register, name, raw, secure));
+
+ /* Load only secure zone if inline-signing is active.
+ * It will not work if raw zone is loaded explicitly
+ * - dns_zone_load() will fail magically. */
+ toview = (secure != NULL) ? secure : raw;
+
+ /*
+ * Zone has to be published *before* zone load
+ * otherwise it will race with zone-view != NULL check
+ * in zone_maintenance() in zone.c.
+ */
+ result = publish_zone(task, inst, toview);
+ if (result != ISC_R_SUCCESS) {
+ dns_zone_log(toview, ISC_LOG_ERROR,
+ cannot add zone to view: %s,
+ dns_result_totext(result));
+ goto cleanup;
+ }
+
+ CHECK(load_zone(toview));
+ if (secure != NULL) {
+ CHECK(zr_get_zone_settings(inst-zone_register, name,
+ zone_settings));
+ CHECK(zone_master_reconfigure_nsec3param(zone_settings,
+ secure));
+ }
+
+cleanup:
+ if (raw != NULL)
+ dns_zone_detach(raw);
+ if (secure != NULL)
+ dns_zone_detach(secure);
+ return result;
+}
+
+/**
* Add all zones in zone register to DNS view specified in inst-view
* and load zones.
*/
isc_result_t
activate_zones(isc_task_t *task, ldap_instance_t *inst) {
isc_result_t result;
- isc_boolean_t loaded;
rbt_iterator_t *iter = NULL;
- dns_zone_t *raw = NULL;
- dns_zone_t *secure = NULL;
- dns_zone_t *toview = NULL;
DECLARE_BUFFERED_NAME(name);
unsigned int published_cnt = 0;
unsigned int total_cnt = 0;
- settings_set_t *zone_settings = NULL;
INIT_BUFFERED_NAME(name);
- CHECK(zr_rbt_iter_init(inst-zone_register, iter, name));
- do {
+ for(result = zr_rbt_iter_init(inst-zone_register, iter, name);
+ result == ISC_R_SUCCESS;
+ dns_name_init(name, NULL), result = rbt_iter_next(iter, name)) {
++total_cnt;
- CHECK(zr_get_zone_ptr(inst-zone_register, name, raw, secure));
- /* Load only secure zone if inline-signing is active.
- * It will not work if raw zone is loaded explicitly. */
- toview = (secure != NULL) ? secure : raw;
- result = load_zone(toview);
- loaded = (result == ISC_R_SUCCESS);
- if (loaded == ISC_FALSE)
- dns_zone_log(raw, ISC_LOG_ERROR,
- unable to load zone: %s,
- dns_result_totext(result));
- else if (secure != NULL) {
- zone_settings = NULL;
- CHECK(zr_get_zone_settings(inst-zone_register,
- name, zone_settings));
- CHECK(zone_master_reconfigure_nsec3param(zone_settings,
- secure));
- }
-
- /*
- * Don't bother if load fails, server will return
- * SERVFAIL for queries beneath this zone. This is
- * admin's problem.
- */
- result = publish_zone(task, inst, toview);
- if (result != ISC_R_SUCCESS)
- dns_zone_log(toview, ISC_LOG_ERROR,
- cannot add zone to view: %s,
- dns_result_totext(result));
- else if (loaded == ISC_TRUE)
+ result = activate_zone(task, inst, name);
+ if (result == ISC_R_SUCCESS)
++published_cnt;
- dns_zone_detach(raw);
- if (secure != NULL)
- dns_zone_detach(secure);
+ };
- INIT_BUFFERED_NAME(name);
- CHECK(rbt_iter_next(iter, name));
- } while (result == ISC_R_SUCCESS);
-
-cleanup:
log_info(%u zones from LDAP instance '%s' loaded
[Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
Hello, Fix race condition during zone loading. DNS zone has to be added to DNS view before dns_zone_load() is called. It is necessary to prevent dns_zone_load() from racing with dns_zone_setview(). This race condition sometimes prevents zone from being signed. Now the unsigned zone is visible until signing process is complete. This mimics BIND behavior for in-line signed zones. https://fedorahosted.org/bind-dyndb-ldap/ticket/56 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
