Commit 8c9fba44 authored by Evan Hunt's avatar Evan Hunt
Browse files

[master] further RPZ fixes

4131.	[bug]		Addressed further problems with reloading RPZ
			zones. [RT #39649]
parent b704d25f
4131. [bug] Addressed further problems with reloading RPZ
zones. [RT #39649]
4130. [bug] The compatability shim for *printf() misprinted some
large numbers. [RT #39586]
......
......@@ -24,6 +24,7 @@
#include <isc/hex.h>
#include <isc/mem.h>
#include <isc/print.h>
#include <isc/rwlock.h>
#include <isc/serial.h>
#include <isc/stats.h>
#include <isc/util.h>
......@@ -4246,9 +4247,6 @@ rpz_rrset_find(ns_client_t *client, dns_name_t *name, dns_rdatatype_t type,
CTRACE(ISC_LOG_DEBUG(3), "rpz_rrset_find");
dns_clientinfomethods_init(&cm, ns_client_sourceip);
dns_clientinfo_init(&ci, client, NULL);
st = client->query.rpz_st;
if ((st->state & DNS_RPZ_RECURSING) != 0) {
INSIST(st->r.r_type == type);
......@@ -4640,19 +4638,6 @@ rpz_rewrite_ip(ns_client_t *client, const isc_netaddr_t *netaddr,
break;
zbits &= (DNS_RPZ_ZMASK(rpz_num) >> 1);
/*
* In case num_zones has changed since zbits was
* originally calculated
*/
if (rpz_num >= rpzs->p.num_zones) {
CTRACE(ISC_LOG_ERROR,
"rpz_rewrite_ip: rpz_num is higher than "
"number of zones");
rpz_clean(&p_zone, &p_db, &p_node, p_rdatasetp);
st->m.policy = DNS_RPZ_POLICY_ERROR;
return (DNS_R_SERVFAIL);
}
/*
* Do not try applying policy zones that cannot replace a
* previously found policy zone.
......@@ -4973,19 +4958,6 @@ rpz_rewrite_name(ns_client_t *client, dns_name_t *trig_name,
if ((zbits & 1) == 0)
continue;
/*
* In case num_zones has changed since the 'have'
* originally calculated
*/
if (rpz_num >= rpzs->p.num_zones) {
CTRACE(ISC_LOG_ERROR,
"rpz_rewrite_name: rpz_num is higher than "
"number of zones");
rpz_clean(&p_zone, &p_db, &p_node, rdatasetp);
st->m.policy = DNS_RPZ_POLICY_ERROR;
return (DNS_R_SERVFAIL);
}
/*
* Do not check policy zones that cannot replace a previously
* found policy.
......@@ -5152,24 +5124,27 @@ rpz_rewrite(ns_client_t *client, dns_rdatatype_t qtype,
(st != NULL && (st->state & DNS_RPZ_REWRITTEN) != 0))
return (DNS_R_DISALLOWED);
LOCK(&rpzs->maint_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
if (rpzs->p.num_zones == 0 ||
(!RECURSIONOK(client) && rpzs->p.no_rd_ok == 0) ||
!rpz_ck_dnssec(client, qresult, ordataset, osigset))
{
UNLOCK(&rpzs->maint_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
return (DNS_R_DISALLOWED);
}
have = rpzs->have;
popt = rpzs->p;
rpz_ver = rpzs->rpz_ver;
UNLOCK(&rpzs->maint_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
if (st == NULL) {
st = isc_mem_get(client->mctx, sizeof(*st));
if (st == NULL)
return (ISC_R_NOMEMORY);
st->state = 0;
}
if (st->state == 0) {
st->state |= DNS_RPZ_ACTIVE;
memset(&st->m, 0, sizeof(st->m));
st->m.type = DNS_RPZ_TYPE_BAD;
st->m.policy = DNS_RPZ_POLICY_MISS;
......
......@@ -662,6 +662,13 @@
corrected. [RT #39481]
</para>
</listitem>
<listitem>
<para>
The server could crash if a reload of an RPZ zone was
initiated while another reload of the same zone was
already in progress. [RT #39649]
</para>
</listitem>
</itemizedlist>
</listitem>
</itemizedlist>
......
......@@ -21,11 +21,12 @@
#define DNS_RPZ_H 1
#include <isc/lang.h>
#include <isc/refcount.h>
#include <isc/rwlock.h>
#include <dns/fixedname.h>
#include <dns/rdata.h>
#include <dns/types.h>
#include <isc/refcount.h>
ISC_LANG_BEGINDECLS
......@@ -227,7 +228,7 @@ struct dns_rpz_zones {
* A second lock for maintenance that guarantees no other thread
* is adding or deleting nodes.
*/
isc_mutex_t search_lock;
isc_rwlock_t search_lock;
isc_mutex_t maint_lock;
dns_rpz_cidr_node_t *cidr;
......@@ -247,6 +248,7 @@ typedef struct {
# define DNS_RPZ_DONE_NSDNAME 0x0010 /* NS name missed; checking addresses */
# define DNS_RPZ_DONE_IPv4 0x0020
# define DNS_RPZ_RECURSING 0x0040
# define DNS_RPZ_ACTIVE 0x0080
/*
* Best match so far.
*/
......
......@@ -1653,8 +1653,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone);
isc_result_t
dns_zonemgr_forcemaint(dns_zonemgr_t *zmgr);
/*%<
* Force zone maintenance of all zones managed by 'zmgr' at its
* earliest convenience.
* Force zone maintenance of all loaded zones managed by 'zmgr'
* to take place at the system's earliest convenience.
*/
void
......
......@@ -1134,8 +1134,10 @@ adjust_quantum(unsigned int old, isc_time_t *start) {
/* Smooth */
new = (new + old * 3) / 4;
isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE,
ISC_LOG_DEBUG(1), "adjust_quantum -> %d", new);
if (new != old)
isc_log_write(dns_lctx, DNS_LOGCATEGORY_DATABASE,
DNS_LOGMODULE_CACHE, ISC_LOG_DEBUG(1),
"adjust_quantum: old=%d, new=%d", old, new);
return (new);
}
......
......@@ -23,6 +23,7 @@
#include <isc/net.h>
#include <isc/netaddr.h>
#include <isc/print.h>
#include <isc/rwlock.h>
#include <isc/stdlib.h>
#include <isc/string.h>
#include <isc/util.h>
......@@ -1390,7 +1391,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
return (ISC_R_NOMEMORY);
memset(new, 0, sizeof(*new));
result = isc_mutex_init(&new->search_lock);
result = isc_rwlock_init(&new->search_lock, 0, 0);
if (result != ISC_R_SUCCESS) {
isc_mem_put(mctx, new, sizeof(*new));
return (result);
......@@ -1398,7 +1399,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
result = isc_mutex_init(&new->maint_lock);
if (result != ISC_R_SUCCESS) {
DESTROYLOCK(&new->search_lock);
isc_rwlock_destroy(&new->search_lock);
isc_mem_put(mctx, new, sizeof(*new));
return (result);
}
......@@ -1406,7 +1407,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
result = isc_refcount_init(&new->refs, 1);
if (result != ISC_R_SUCCESS) {
DESTROYLOCK(&new->maint_lock);
DESTROYLOCK(&new->search_lock);
isc_rwlock_destroy(&new->search_lock);
isc_mem_put(mctx, new, sizeof(*new));
return (result);
}
......@@ -1416,7 +1417,7 @@ dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, isc_mem_t *mctx) {
isc_refcount_decrement(&new->refs, NULL);
isc_refcount_destroy(&new->refs);
DESTROYLOCK(&new->maint_lock);
DESTROYLOCK(&new->search_lock);
isc_rwlock_destroy(&new->search_lock);
isc_mem_put(mctx, new, sizeof(*new));
return (result);
}
......@@ -1535,7 +1536,7 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) {
cidr_free(rpzs);
dns_rbt_destroy(&rpzs->rbt);
DESTROYLOCK(&rpzs->maint_lock);
DESTROYLOCK(&rpzs->search_lock);
isc_rwlock_destroy(&rpzs->search_lock);
isc_refcount_destroy(&rpzs->refs);
isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs));
}
......@@ -1580,7 +1581,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
*/
tgt = DNS_RPZ_ZBIT(rpz_num);
LOCK(&rpzs->maint_lock);
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
if ((rpzs->load_begun & tgt) == 0) {
/*
* There is no existing version of the target zone.
......@@ -1604,7 +1605,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
isc_refcount_increment(&rpz->refs, NULL);
}
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
UNLOCK(&rpzs->maint_lock);
return (ISC_R_SUCCESS);
......@@ -1612,7 +1613,7 @@ dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp,
/*
* This function updates "have" bits and also the qname_skip_recurse
* mask. It must be called when holding rpzs->search_lock.
* mask. It must be called when holding a write lock on rpzs->search_lock.
*/
static void
fix_triggers(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num) {
......@@ -1740,16 +1741,16 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
* This is a successful initial zone loading, perhaps
* for a new instance of a view.
*/
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
fix_triggers(rpzs, rpz_num);
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
UNLOCK(&rpzs->maint_lock);
dns_rpz_detach_rpzs(load_rpzsp);
return (ISC_R_SUCCESS);
}
LOCK(&load_rpzs->maint_lock);
LOCK(&load_rpzs->search_lock);
RWLOCK(&load_rpzs->search_lock, isc_rwlocktype_write);
/*
* Unless there is only one policy zone, copy the other policy zones
......@@ -1848,7 +1849,7 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
/*
* Exchange the summary databases.
*/
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
rpzs->triggers[rpz_num] = load_rpzs->triggers[rpz_num];
fix_triggers(rpzs, rpz_num);
......@@ -1861,13 +1862,13 @@ dns_rpz_ready(dns_rpz_zones_t *rpzs,
rpzs->rbt = load_rpzs->rbt;
load_rpzs->rbt = rbt;
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
result = ISC_R_SUCCESS;
unlock_and_detach:
UNLOCK(&rpzs->maint_lock);
UNLOCK(&load_rpzs->search_lock);
RWUNLOCK(&load_rpzs->search_lock, isc_rwlocktype_write);
UNLOCK(&load_rpzs->maint_lock);
dns_rpz_detach_rpzs(load_rpzsp);
return (result);
......@@ -1890,7 +1891,7 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_name_t *src_name)
rpz_type = type_from_name(rpz, src_name);
LOCK(&rpzs->maint_lock);
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
switch (rpz_type) {
case DNS_RPZ_TYPE_QNAME:
......@@ -1906,7 +1907,7 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_name_t *src_name)
break;
}
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
UNLOCK(&rpzs->maint_lock);
return (result);
}
......@@ -2096,7 +2097,7 @@ dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num,
rpz_type = type_from_name(rpz, src_name);
LOCK(&rpzs->maint_lock);
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_write);
switch (rpz_type) {
case DNS_RPZ_TYPE_QNAME:
......@@ -2112,7 +2113,7 @@ dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num,
break;
}
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_write);
UNLOCK(&rpzs->maint_lock);
}
......@@ -2197,13 +2198,13 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
return (DNS_RPZ_INVALID_NUM);
make_addr_set(&tgt_set, zbits, rpz_type);
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
result = search(rpzs, &tgt_ip, 128, &tgt_set, ISC_FALSE, &found);
if (result == ISC_R_NOTFOUND) {
/*
* There are no eligible zones for this IP address.
*/
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
return (DNS_RPZ_INVALID_NUM);
}
......@@ -2227,7 +2228,7 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
break;
}
result = ip2name(&found->ip, found->prefix, dns_rootname, ip_name);
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
if (result != ISC_R_SUCCESS) {
/*
* bin/tests/system/rpz/tests.sh looks for "rpz.*failed".
......@@ -2260,7 +2261,7 @@ dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
found_zbits = 0;
LOCK(&rpzs->search_lock);
RWLOCK(&rpzs->search_lock, isc_rwlocktype_read);
nmnode = NULL;
result = dns_rbt_findnode(rpzs->rbt, trig_name, NULL, &nmnode, NULL,
......@@ -2304,7 +2305,7 @@ dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type,
break;
}
UNLOCK(&rpzs->search_lock);
RWUNLOCK(&rpzs->search_lock, isc_rwlocktype_read);
return (zbits & found_zbits);
}
......
......@@ -5752,7 +5752,6 @@ dns_zone_setdb(dns_zone_t *zone, dns_db_t *db) {
/*
* Co-ordinates the starting of routine jobs.
*/
void
dns_zone_maintenance(dns_zone_t *zone) {
const char me[] = "dns_zone_maintenance";
......@@ -12296,8 +12295,9 @@ zone_settimer(dns_zone_t *zone, isc_time_t *now) {
isc_time_t next;
isc_result_t result;
ENTER;
REQUIRE(DNS_ZONE_VALID(zone));
ENTER;
if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
return;
......@@ -12361,12 +12361,12 @@ zone_settimer(dns_zone_t *zone, isc_time_t *now) {
if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING)) {
INSIST(!isc_time_isepoch(&zone->refreshtime));
if (isc_time_isepoch(&next) ||
isc_time_compare(&zone->refreshtime, &next) < 0)
next = zone->refreshtime;
}
!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING) &&
!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING) &&
!isc_time_isepoch(&zone->refreshtime) &&
(isc_time_isepoch(&next) ||
isc_time_compare(&zone->refreshtime, &next) < 0))
next = zone->refreshtime;
if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED) &&
!isc_time_isepoch(&zone->expiretime)) {
if (isc_time_isepoch(&next) ||
......
......@@ -314,12 +314,15 @@ asyncload(dns_zone_t *zone, void *callback) {
zt = dns_zone_getview(zone)->zonetable;
INSIST(VALID_ZT(zt));
INSIST(zt->references > 0);
zt->references++;
zt->loads_pending++;
result = dns_zone_asyncload(zone, *loaded, zt);
if (result == ISC_R_SUCCESS) {
if (result != ISC_R_SUCCESS) {
zt->references--;
zt->loads_pending--;
INSIST(zt->references > 0);
zt->references++;
INSIST(zt->references != 0);
zt->loads_pending++;
}
return (ISC_R_SUCCESS);
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment