ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

4374. [bug] Use SAVE/RESTORE macros in query.c to reduce the

Browse Source 
probability of reference counting errors as seen
                        in 4365. [RT #42405]
This commit is contained in:
Mark Andrews
2016-05-26 12:11:00 +10:00
parent ede52ba98d
commit ac11084829
2 changed files with 99 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGES
View File

@@ -1,3 +1,7 @@
4374. [bug] Use SAVE/RESTORE macros in query.c to reduce the
probability of reference counting errors as seen
in 4365. [RT #42405]
4373. [bug] Address undefined behaviour in getaddrinfo. [RT #42479]
4372. [bug] Address undefined behaviour in libt_api. [RT #42480]

232
bin/named/query.c
View File

@@ -178,6 +178,19 @@ client_trace(ns_client_t *client, int level, const char *message) {
#define SFCACHE_CDFLAG 0x1
/*
* These have the same semantics as:
*
* foo_attach(b, a);
* foo_detach(&a);
*
* without the locking and magic testing.
*
* We use SAVE and RESTORE as that shows the operation being performed.
*/
#define SAVE(a, b) do { INSIST(a == NULL); a = b; b = NULL; } while (0)
#define RESTORE(a, b) SAVE(a, b)
typedef struct client_additionalctx {
ns_client_t *client;
dns_rdataset_t *rdataset;
@@ -3242,16 +3255,12 @@ query_addbestns(ns_client_t *client) {
goto cleanup;
if (USECACHE(client)) {
query_keepname(client, fname, dbuf);
zdb = db;
zfname = fname;
fname = NULL;
zrdataset = rdataset;
rdataset = NULL;
zsigrdataset = sigrdataset;
sigrdataset = NULL;
dns_db_detachnode(db, &node);
SAVE(zdb, db);
SAVE(zfname, fname);
SAVE(zrdataset, rdataset);
SAVE(zsigrdataset, sigrdataset);
version = NULL;
db = NULL;
dns_db_attach(client->view->cachedb, &db);
is_zone = ISC_FALSE;
goto db_find;
@@ -3282,8 +3291,6 @@ query_addbestns(ns_client_t *client) {
if (use_zone) {
query_releasename(client, &fname);
fname = zfname;
zfname = NULL;
/*
* We've already done query_keepname() on
* zfname, so we must set dbuf to NULL to
@@ -3294,10 +3301,10 @@ query_addbestns(ns_client_t *client) {
query_putrdataset(client, &rdataset);
if (sigrdataset != NULL)
query_putrdataset(client, &sigrdataset);
rdataset = zrdataset;
zrdataset = NULL;
sigrdataset = zsigrdataset;
zsigrdataset = NULL;
RESTORE(fname, zfname);
RESTORE(rdataset, zrdataset);
RESTORE(sigrdataset, zsigrdataset);
}
/*
@@ -4646,7 +4653,7 @@ rpz_save_p(dns_rpz_st_t *st, dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type,
dns_dbnode_t **nodep, dns_rdataset_t **rdatasetp,
dns_dbversion_t *version)
{
dns_rdataset_t *trdataset;
dns_rdataset_t *trdataset = NULL;
rpz_match_clear(st);
st->m.rpz = rpz;
@@ -4655,25 +4662,22 @@ rpz_save_p(dns_rpz_st_t *st, dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type,
dns_name_copy(p_name, st->p_name, NULL);
st->m.prefix = prefix;
st->m.result = result;
st->m.zone = *zonep;
*zonep = NULL;
st->m.db = *dbp;
*dbp = NULL;
st->m.node = *nodep;
*nodep = NULL;
SAVE(st->m.zone, *zonep);
SAVE(st->m.db, *dbp);
SAVE(st->m.node, *nodep);
if (*rdatasetp != NULL && dns_rdataset_isassociated(*rdatasetp)) {
/*
* Save the replacement rdataset from the policy
* and make the previous replacement rdataset scratch.
*/
trdataset = st->m.rdataset;
st->m.rdataset = *rdatasetp;
*rdatasetp = trdataset;
SAVE(trdataset, st->m.rdataset);
SAVE(st->m.rdataset, *rdatasetp);
SAVE(*rdatasetp, trdataset);
st->m.ttl = ISC_MIN(st->m.rdataset->ttl, rpz->max_policy_ttl);
} else {
st->m.ttl = ISC_MIN(DNS_RPZ_TTL_DEFAULT, rpz->max_policy_ttl);
}
st->m.version = version;
SAVE(st->m.version, version);
}
/*
@@ -6165,12 +6169,12 @@ dns64_aaaaok(ns_client_t *client, dns_rdataset_t *rdataset,
aaaaok, count)) {
for (i = 0; i < count; i++) {
if (aaaaok != NULL && !aaaaok[i]) {
client->query.dns64_aaaaok = aaaaok;
SAVE(client->query.dns64_aaaaok, aaaaok);
client->query.dns64_aaaaoklen = count;
break;
}
}
if (i == count && aaaaok != NULL)
if (aaaaok != NULL)
isc_mem_put(client->mctx, aaaaok,
sizeof(isc_boolean_t) * count);
return (ISC_TRUE);
@@ -6612,16 +6616,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
is_zone = rpz_st->q.is_zone;
authoritative = rpz_st->q.authoritative;
zone = rpz_st->q.zone;
rpz_st->q.zone = NULL;
node = rpz_st->q.node;
rpz_st->q.node = NULL;
db = rpz_st->q.db;
rpz_st->q.db = NULL;
rdataset = rpz_st->q.rdataset;
rpz_st->q.rdataset = NULL;
sigrdataset = rpz_st->q.sigrdataset;
rpz_st->q.sigrdataset = NULL;
RESTORE(zone, rpz_st->q.zone);
RESTORE(node, rpz_st->q.node);
RESTORE(db, rpz_st->q.db);
RESTORE(rdataset, rpz_st->q.rdataset);
RESTORE(sigrdataset, rpz_st->q.sigrdataset);
qtype = rpz_st->q.qtype;
rpz_st->r.db = event->db;
@@ -6649,16 +6648,12 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
#endif
qtype = client->query.redirect.qtype;
INSIST(client->query.redirect.rdataset != NULL);
rdataset = client->query.redirect.rdataset;
client->query.redirect.rdataset = NULL;
sigrdataset = client->query.redirect.sigrdataset;
client->query.redirect.sigrdataset = NULL;
db = client->query.redirect.db;
client->query.redirect.db = NULL;
node = client->query.redirect.node;
client->query.redirect.node = NULL;
zone = client->query.redirect.zone;
client->query.redirect.zone = NULL;
RESTORE(rdataset, client->query.redirect.rdataset);
RESTORE(sigrdataset,
client->query.redirect.sigrdataset);
RESTORE(db, client->query.redirect.db);
RESTORE(node, client->query.redirect.node);
RESTORE(zone, client->query.redirect.zone);
authoritative = client->query.redirect.authoritative;
/*
@@ -6880,9 +6875,10 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
dns_db_detach(&db);
if (zone != NULL)
dns_zone_detach(&zone);
version = tversion;
db = tdb;
zone = tzone;
version = NULL;
RESTORE(version, tversion);
RESTORE(db, tdb);
RESTORE(zone, tzone);
is_zone = ISC_TRUE;
result = ISC_R_SUCCESS;
} else {
@@ -7140,16 +7136,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
rpz_st->q.qtype = qtype;
rpz_st->q.is_zone = is_zone;
rpz_st->q.authoritative = authoritative;
rpz_st->q.zone = zone;
zone = NULL;
rpz_st->q.db = db;
db = NULL;
rpz_st->q.node = node;
node = NULL;
rpz_st->q.rdataset = rdataset;
rdataset = NULL;
rpz_st->q.sigrdataset = sigrdataset;
sigrdataset = NULL;
SAVE(rpz_st->q.zone, zone);
SAVE(rpz_st->q.db, db);
SAVE(rpz_st->q.node, node);
SAVE(rpz_st->q.rdataset, rdataset);
SAVE(rpz_st->q.sigrdataset, sigrdataset);
dns_name_copy(fname, rpz_st->fname, NULL);
rpz_st->q.result = result;
client->query.attributes |= NS_QUERYATTR_RECURSING;
@@ -7179,20 +7170,17 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
rpz_clean(&zone, &db, &node, NULL);
if (rpz_st->m.rdataset != NULL) {
query_putrdataset(client, &rdataset);
rdataset = rpz_st->m.rdataset;
rpz_st->m.rdataset = NULL;
RESTORE(rdataset, rpz_st->m.rdataset);
} else if (rdataset != NULL &&
dns_rdataset_isassociated(rdataset)) {
dns_rdataset_disassociate(rdataset);
}
node = rpz_st->m.node;
rpz_st->m.node = NULL;
db = rpz_st->m.db;
rpz_st->m.db = NULL;
version = rpz_st->m.version;
rpz_st->m.version = NULL;
zone = rpz_st->m.zone;
rpz_st->m.zone = NULL;
version = NULL;
RESTORE(node, rpz_st->m.node);
RESTORE(db, rpz_st->m.db);
RESTORE(version, rpz_st->m.version);
RESTORE(zone, rpz_st->m.zone);
switch (rpz_st->m.policy) {
case DNS_RPZ_POLICY_TCP_ONLY:
@@ -7406,9 +7394,10 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
dns_db_detach(&db);
if (zone != NULL)
dns_zone_detach(&zone);
version = tversion;
db = tdb;
zone = tzone;
version = NULL;
RESTORE(version, tversion);
RESTORE(db, tdb);
RESTORE(zone, tzone);
authoritative = ISC_TRUE;
goto db_find;
}
@@ -7471,17 +7460,12 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
* use it instead.
*/
query_keepname(client, fname, dbuf);
zdb = db;
zfname = fname;
fname = NULL;
zrdataset = rdataset;
rdataset = NULL;
zsigrdataset = sigrdataset;
sigrdataset = NULL;
dns_db_detachnode(db, &node);
zversion = version;
version = NULL;
db = NULL;
SAVE(zdb, db);
SAVE(zfname, fname);
SAVE(zrdataset, rdataset);
SAVE(zsigrdataset, sigrdataset);
SAVE(zversion, version);
dns_db_attach(client->view->cachedb, &db);
is_zone = ISC_FALSE;
goto db_find;
@@ -7506,8 +7490,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
* static-stub zone.
*/
query_releasename(client, &fname);
fname = zfname;
zfname = NULL;
/*
* We've already done query_keepname() on
* zfname, so we must set dbuf to NULL to
@@ -7519,12 +7501,12 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
if (sigrdataset != NULL)
query_putrdataset(client,
&sigrdataset);
rdataset = zrdataset;
zrdataset = NULL;
sigrdataset = zsigrdataset;
zsigrdataset = NULL;
version = zversion;
zversion = NULL;
version = NULL;
RESTORE(fname, zfname);
RESTORE(rdataset, zrdataset);
RESTORE(sigrdataset, zsigrdataset);
RESTORE(version, zversion);
/*
* We don't clean up zdb here because we
* may still need it. It will get cleaned
@@ -7620,10 +7602,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
query_putrdataset(client, &rdataset);
if (sigrdataset != NULL)
query_putrdataset(client, &sigrdataset);
rdataset = client->query.dns64_aaaa;
sigrdataset = client->query.dns64_sigaaaa;
client->query.dns64_aaaa = NULL;
client->query.dns64_sigaaaa = NULL;
RESTORE(rdataset, client->query.dns64_aaaa);
RESTORE(sigrdataset, client->query.dns64_sigaaaa);
if (fname == NULL) {
dbuf = query_getnamebuf(client);
if (dbuf == NULL) {
@@ -7660,15 +7640,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
* Look to see if there are A records for this
* name.
*/
INSIST(client->query.dns64_aaaa == NULL);
INSIST(client->query.dns64_sigaaaa == NULL);
client->query.dns64_aaaa = rdataset;
client->query.dns64_sigaaaa = sigrdataset;
SAVE(client->query.dns64_aaaa, rdataset);
SAVE(client->query.dns64_sigaaaa, sigrdataset);
client->query.dns64_ttl = dns64_ttl(db, version);
query_releasename(client, &fname);
dns_db_detachnode(db, &node);
rdataset = NULL;
sigrdataset = NULL;
type = qtype = dns_rdatatype_a;
rpz_st = client->query.rpz_st;
if (rpz_st != NULL) {
@@ -7837,23 +7813,18 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
dns_nsstatscounter_nxdomainredirect_rlookup);
client->query.redirect.qtype = qtype;
INSIST(rdataset != NULL);
client->query.redirect.rdataset = rdataset;
client->query.redirect.sigrdataset =
sigrdataset;
client->query.redirect.db = db;
client->query.redirect.node = node;
client->query.redirect.zone = zone;
SAVE(client->query.redirect.rdataset, rdataset);
SAVE(client->query.redirect.sigrdataset,
sigrdataset);
SAVE(client->query.redirect.db, db);
SAVE(client->query.redirect.node, node);
SAVE(client->query.redirect.zone, zone);
client->query.redirect.result = DNS_R_NXDOMAIN;
dns_name_copy(fname,
client->query.redirect.fname,
NULL);
client->query.redirect.authoritative =
authoritative;
db = NULL;
node = NULL;
zone = NULL;
rdataset = NULL;
sigrdataset = NULL;
goto cleanup;
}
if (tresult == ISC_R_SUCCESS) {
@@ -7955,22 +7926,17 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
if (tresult == DNS_R_CONTINUE) {
inc_stats(client,
dns_nsstatscounter_nxdomainredirect_rlookup);
client->query.redirect.db = db;
client->query.redirect.node = node;
client->query.redirect.zone = zone;
SAVE(client->query.redirect.db, db);
SAVE(client->query.redirect.node, node);
SAVE(client->query.redirect.zone, zone);
client->query.redirect.qtype = qtype;
INSIST(rdataset != NULL);
client->query.redirect.rdataset = rdataset;
client->query.redirect.sigrdataset = sigrdataset;
SAVE(client->query.redirect.rdataset, rdataset);
SAVE(client->query.redirect.sigrdataset, sigrdataset);
client->query.redirect.result = DNS_R_NCACHENXDOMAIN;
dns_name_copy(fname, client->query.redirect.fname,
NULL);
client->query.redirect.authoritative = authoritative;
db = NULL;
node = NULL;
zone = NULL;
rdataset = NULL;
sigrdataset = NULL;
goto cleanup;
}
if (tresult == ISC_R_SUCCESS) {
@@ -8021,10 +7987,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
query_putrdataset(client, &rdataset);
if (sigrdataset != NULL)
query_putrdataset(client, &sigrdataset);
rdataset = client->query.dns64_aaaa;
sigrdataset = client->query.dns64_sigaaaa;
client->query.dns64_aaaa = NULL;
client->query.dns64_sigaaaa = NULL;
RESTORE(rdataset, client->query.dns64_aaaa);
RESTORE(sigrdataset, client->query.dns64_sigaaaa);
if (fname == NULL) {
dbuf = query_getnamebuf(client);
if (dbuf == NULL) {
@@ -8058,10 +8022,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
* Look to see if there are A records for this
* name.
*/
INSIST(client->query.dns64_aaaa == NULL);
INSIST(client->query.dns64_sigaaaa == NULL);
client->query.dns64_aaaa = rdataset;
client->query.dns64_sigaaaa = sigrdataset;
/*
* If the ttl is zero we need to workout if we have just
* decremented to zero or if there was no negative cache
@@ -8071,11 +8032,10 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
client->query.dns64_ttl = rdataset->ttl;
else if (dns_rdataset_first(rdataset) == ISC_R_SUCCESS)
client->query.dns64_ttl = 0;
SAVE(client->query.dns64_aaaa, rdataset);
SAVE(client->query.dns64_sigaaaa, sigrdataset);
query_releasename(client, &fname);
dns_db_detachnode(db, &node);
rdataset = NULL;
sigrdataset = NULL;
fname = NULL;
type = qtype = dns_rdatatype_a;
rpz_st = client->query.rpz_st;
if (rpz_st != NULL) {
@@ -8643,13 +8603,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
* Look to see if there are A records for this
* name.
*/
client->query.dns64_aaaa = rdataset;
client->query.dns64_sigaaaa = sigrdataset;
client->query.dns64_ttl = rdataset->ttl;
SAVE(client->query.dns64_aaaa, rdataset);
SAVE(client->query.dns64_sigaaaa, sigrdataset);
query_releasename(client, &fname);
dns_db_detachnode(db, &node);
rdataset = NULL;
sigrdataset = NULL;
type = qtype = dns_rdatatype_a;
rpz_st = client->query.rpz_st;
if (rpz_st != NULL) {