On 9.4.2014 15:20, Tomas Hozza wrote:
On 04/09/2014 02:07 PM, Petr Spacek wrote:
Hello,
Prevent NULL dereference before sync_concurr_limit_signal() calls.
Missing check was causing NULL dereference in case where
manager_get_ldap_instance() failed. This typically happens when BIND
is processing LDAP updates during shutdown.
I noticed this crash during sanity testing 4.2 release...
Please review it ASAP so I can release 4.3.
How to reproduce the problem:
Run BIND manually from console:
$ named -4 -g -u named -m record -n 10
and press Ctrl+C "almost immediately".
Sometimes it shutdowns cleanly and sometimes you can see a crash:
Thank you for your time!
ACK.
I'm not able to reproduce the issue, but the patch looks reasonable and
should not break anything.
Thanks. I have modified the patch once again before push to silence Clang
warnings about potential NULL-inst deference on following lines:
if (dns_name_dynamic(&name))
dns_name_free(&name, inst->mctx);
In reality the NULL dereference cannot happen because it is guarded by
condition in dns_name_dynamic().
This is not a problem now but the behavior depends on internal implementation
in BIND. It is definitely better to add explicit check to stay safe ...
--
Petr^2 Spacek
From cb9ed81e6b1fdd26a06739fe819b730eb4a70839 Mon Sep 17 00:00:00 2001
From: Petr Spacek <[email protected]>
Date: Wed, 9 Apr 2014 14:01:00 +0200
Subject: [PATCH] Prevent NULL dereference before sync_concurr_limit_signal()
calls.
Missing check was causing NULL dereference in case where
manager_get_ldap_instance() failed. This typically happens when BIND
is processing LDAP updates during shutdown.
Signed-off-by: Petr Spacek <[email protected]>
---
NEWS | 5 +++++
src/ldap_helper.c | 35 ++++++++++++++++++++---------------
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/NEWS b/NEWS
index e787e7f2d73e3e99d3d5c0d03b9ea92dff75b510..78843f2d036a7ab25c00924af91e63228aea0a8f 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,8 @@
+4.3
+====
+[1] LDAP update processing was fixed to prevent BIND from crashing during
+ shutdown.
+
4.2
====
[1] Record parsing was fixed to prevent child-zone data corruption in cases
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 678e9f8a52181a5c63c96d29da9b3e5ec3b1273d..cf8d45ecc6ef24653be731020703c24c9b4c7214 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3913,16 +3913,18 @@ update_zone(isc_task_t *task, isc_event_t *event)
}
cleanup:
- sync_concurr_limit_signal(inst->sctx);
+ if (inst != NULL) {
+ sync_concurr_limit_signal(inst->sctx);
+ if (dns_name_dynamic(&currname))
+ dns_name_free(&currname, inst->mctx);
+ if (dns_name_dynamic(&prevname))
+ dns_name_free(&prevname, inst->mctx);
+ }
if (result != ISC_R_SUCCESS)
log_error_r("update_zone (syncrepl) failed for '%s'. "
"Zones can be outdated, run `rndc reload`",
pevent->dn);
- if (dns_name_dynamic(&currname))
- dns_name_free(&currname, inst->mctx);
- if (dns_name_dynamic(&prevname))
- dns_name_free(&prevname, inst->mctx);
isc_mem_free(mctx, pevent->dbname);
if (pevent->prevdn != NULL)
isc_mem_free(mctx, pevent->prevdn);
@@ -3948,7 +3950,8 @@ update_config(isc_task_t *task, isc_event_t *event)
CHECK(ldap_parse_configentry(entry, inst));
cleanup:
- sync_concurr_limit_signal(inst->sctx);
+ if (inst != NULL)
+ sync_concurr_limit_signal(inst->sctx);
if (result != ISC_R_SUCCESS)
log_error_r("update_config (syncrepl) failed for '%s'. "
"Configuration can be outdated, run `rndc reload`",
@@ -4200,17 +4203,19 @@ cleanup:
pevent->dn, pevent->chgtype);
}
- sync_concurr_limit_signal(inst->sctx);
+ if (inst != NULL) {
+ sync_concurr_limit_signal(inst->sctx);
+ if (dns_name_dynamic(&name))
+ dns_name_free(&name, inst->mctx);
+ if (dns_name_dynamic(&prevname))
+ dns_name_free(&prevname, inst->mctx);
+ if (dns_name_dynamic(&origin))
+ dns_name_free(&origin, inst->mctx);
+ if (dns_name_dynamic(&prevorigin))
+ dns_name_free(&prevorigin, inst->mctx);
+ }
if (zone_ptr != NULL)
dns_zone_detach(&zone_ptr);
- if (dns_name_dynamic(&name))
- dns_name_free(&name, inst->mctx);
- if (dns_name_dynamic(&prevname))
- dns_name_free(&prevname, inst->mctx);
- if (dns_name_dynamic(&origin))
- dns_name_free(&origin, inst->mctx);
- if (dns_name_dynamic(&prevorigin))
- dns_name_free(&prevorigin, inst->mctx);
ldapdb_rdatalist_destroy(mctx, &rdatalist);
isc_mem_free(mctx, pevent->dbname);
if (pevent->prevdn != NULL)
--
1.9.0
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
