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

probability of reference counting errors as seen
                        in 4365. [RT #42405]

(cherry picked from commit ac11084829)
This commit is contained in:
Mark Andrews 2016-05-26 12:11:00 +10:00
parent 59300548a7
commit 900a63cb04
2 changed files with 91 additions and 116 deletions

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]

View file

@ -165,6 +165,19 @@ client_trace(ns_client_t *client, int level, const char *message) {
#define PENDINGOK(x) (((x) & DNS_DBFIND_PENDINGOK) != 0)
/*
* 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;
@ -1436,8 +1449,7 @@ query_addadditional(void *arg, dns_name_t *name, dns_rdatatype_t qtype) {
} else
need_addname = ISC_TRUE;
ISC_LIST_APPEND(fname->list, rdataset, link);
trdataset = rdataset;
rdataset = NULL;
SAVE(trdataset, rdataset);
added_something = ISC_TRUE;
/*
* Note: we only add SIGs if we've added the type they cover,
@ -3164,16 +3176,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;
@ -3204,8 +3212,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
@ -3216,10 +3222,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);
}
/*
@ -4055,8 +4061,7 @@ rpz_rrset_find(ns_client_t *client, dns_rpz_type_t rpz_type,
st->r.db = NULL;
if (*rdatasetp != NULL)
query_putrdataset(client, rdatasetp);
*rdatasetp = st->r.r_rdataset;
st->r.r_rdataset = NULL;
RESTORE(*rdatasetp, st->r.r_rdataset);
result = st->r.r_result;
if (result == DNS_R_DELEGATION) {
CTRACE(ISC_LOG_ERROR, "RPZ recursing");
@ -4470,6 +4475,7 @@ rpz_rewrite_name(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
zone = NULL;
db = NULL;
node = NULL;
version = NULL;
for (rpz = ISC_LIST_HEAD(client->view->rpz_zones);
rpz != NULL;
@ -4605,24 +4611,21 @@ rpz_rewrite_name(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
dns_name_copy(rpz_qname, st->qname, NULL);
if (*rdatasetp != NULL &&
dns_rdataset_isassociated(*rdatasetp)) {
dns_rdataset_t *trdataset;
dns_rdataset_t *trdataset = NULL;
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.node = node;
node = NULL;
st->m.db = db;
db = NULL;
st->m.version = version;
st->m.zone = zone;
zone = NULL;
SAVE(st->m.node, node);
SAVE(st->m.db, db);
SAVE(st->m.version, version);
SAVE(st->m.zone, zone);
}
}
@ -5499,12 +5502,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);
@ -5778,32 +5781,27 @@ 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;
if (event->node != NULL)
dns_db_detachnode(event->db, &event->node);
SAVE(rpz_st->r.db, event->db);
rpz_st->r.r_type = event->qtype;
rpz_st->r.r_rdataset = event->rdataset;
SAVE(rpz_st->r.r_rdataset, event->rdataset);
query_putrdataset(client, &event->sigrdataset);
} else {
authoritative = ISC_FALSE;
qtype = event->qtype;
db = event->db;
node = event->node;
rdataset = event->rdataset;
sigrdataset = event->sigrdataset;
RESTORE(db, event->db);
RESTORE(node, event->node);
RESTORE(rdataset, event->rdataset);
RESTORE(sigrdataset, event->sigrdataset);
}
INSIST(rdataset != NULL);
@ -5934,9 +5932,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 {
@ -6173,16 +6172,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;
@ -6204,20 +6198,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_NXDOMAIN:
@ -6408,9 +6399,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;
}
@ -6473,17 +6465,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;
@ -6508,8 +6495,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
@ -6521,12 +6506,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
@ -6621,10 +6606,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) {
@ -6661,15 +6644,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) {
@ -6938,10 +6917,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) {
@ -6975,10 +6952,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
@ -6988,11 +6962,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) {
@ -7508,13 +7481,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) {