mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 05:57:52 +00:00
Code Issues Releases Wiki Activity

fix: dev: Lock and attach when returning zone stats

Browse Source 
When returning zone statistics counters, the statistics sets are now attached while the zone is locked.  This addresses Coverity warnings CID 468720, 468728 and 468729.

Closes #4934

Merge branch '4934-lock-and-attach-when-return-zone-stats' into 'main'

See merge request isc-projects/bind9!9488
This commit is contained in:
Mark Andrews 2024-12-06 05:32:05 +00:00
commit 3c720c6425
7 changed files with 107 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
bin/named/statschannel.c
View File

@ -1301,6 +1301,9 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
stats_dumparg_t dumparg;
const char *ztype;
isc_time_t timestamp;
isc_stats_t *zonestats = NULL;
dns_stats_t *rcvquerystats = NULL;
dns_stats_t *dnssecsignstats = NULL;
statlevel = dns_zone_getstatlevel(zone);
if (statlevel == dns_zonestat_none) {
@ -1365,14 +1368,11 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
}
if (statlevel == dns_zonestat_full) {
isc_stats_t *zonestats;
isc_stats_t *gluecachestats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
uint64_t nsstat_values[ns_statscounter_max];
uint64_t gluecachestats_values[dns_gluecachestatscounter_max];
zonestats = dns_zone_getrequeststats(zone);
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
TRY0(xmlTextWriterStartElement(writer,
ISC_XMLCHAR "counters"));
@ -1386,6 +1386,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
nsstat_values, ISC_STATSDUMP_VERBOSE));
/* counters type="rcode"*/
TRY0(xmlTextWriterEndElement(writer));
isc_stats_detach(&zonestats);
}
gluecachestats = dns_zone_getgluecachestats(zone);
@ -1406,7 +1407,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
TRY0(xmlTextWriterEndElement(writer));
}
rcvquerystats = dns_zone_getrcvquerystats(zone);
dns_zone_getrcvquerystats(zone, &rcvquerystats);
if (rcvquerystats != NULL) {
TRY0(xmlTextWriterStartElement(writer,
ISC_XMLCHAR "counters"));
@ -1421,9 +1422,10 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
/* counters type="qtype"*/
TRY0(xmlTextWriterEndElement(writer));
dns_stats_detach(&rcvquerystats);
}
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dns_zone_getdnssecsignstats(zone, &dnssecsignstats);
if (dnssecsignstats != NULL) {
/* counters type="dnssec-sign"*/
TRY0(xmlTextWriterStartElement(writer,
@ -1456,6 +1458,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
/* counters type="dnssec-refresh"*/
TRY0(xmlTextWriterEndElement(writer));
dns_stats_detach(&dnssecsignstats);
}
}
@ -1463,6 +1466,15 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
return ISC_R_SUCCESS;
cleanup:
if (zonestats != NULL) {
isc_stats_detach(&zonestats);
}
if (rcvquerystats != NULL) {
dns_stats_detach(&rcvquerystats);
}
if (dnssecsignstats != NULL) {
dns_stats_detach(&dnssecsignstats);
}
isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
ISC_LOG_ERROR, "Failed at zone_xmlrender()");
return ISC_R_FAILURE;
@ -2343,6 +2355,9 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
json_object *zoneobj = NULL;
dns_zonestat_level_t statlevel;
isc_time_t timestamp;
isc_stats_t *zonestats = NULL;
dns_stats_t *rcvquerystats = NULL;
dns_stats_t *dnssecsignstats = NULL;
statlevel = dns_zone_getstatlevel(zone);
if (statlevel == dns_zonestat_none) {
@ -2392,14 +2407,11 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
}
if (statlevel == dns_zonestat_full) {
isc_stats_t *zonestats;
isc_stats_t *gluecachestats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
uint64_t nsstat_values[ns_statscounter_max];
uint64_t gluecachestats_values[dns_gluecachestatscounter_max];
zonestats = dns_zone_getrequeststats(zone);
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
json_object *counters = json_object_new_object();
if (counters == NULL) {
@ -2450,7 +2462,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
}
}
rcvquerystats = dns_zone_getrcvquerystats(zone);
dns_zone_getrcvquerystats(zone, &rcvquerystats);
if (rcvquerystats != NULL) {
stats_dumparg_t dumparg;
json_object *counters = json_object_new_object();
@ -2474,7 +2486,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
}
}
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dns_zone_getdnssecsignstats(zone, &dnssecsignstats);
if (dnssecsignstats != NULL) {
stats_dumparg_t dumparg;
json_object *sign_counters = json_object_new_object();
@ -2530,6 +2542,15 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
result = ISC_R_SUCCESS;
cleanup:
if (zonestats != NULL) {
isc_stats_detach(&zonestats);
}
if (rcvquerystats != NULL) {
dns_stats_detach(&rcvquerystats);
}
if (dnssecsignstats != NULL) {
dns_stats_detach(&dnssecsignstats);
}
if (zoneobj != NULL) {
json_object_put(zoneobj);
}
@ -4102,12 +4123,14 @@ named_stats_dump(named_server_t *server, FILE *fp) {
result == ISC_R_SUCCESS;
next = NULL, result = dns_zone_next(zone, &next), zone = next)
{
isc_stats_t *zonestats = dns_zone_getrequeststats(zone);
isc_stats_t *zonestats = NULL;
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
char zonename[DNS_NAME_FORMATSIZE];
view = dns_zone_getview(zone);
if (view == NULL) {
isc_stats_detach(&zonestats);
continue;
}
@ -4123,6 +4146,7 @@ named_stats_dump(named_server_t *server, FILE *fp) {
NULL, nsstats_desc,
ns_statscounter_max, nsstats_index,
nsstat_values, 0);
isc_stats_detach(&zonestats);
}
}

13
lib/dns/include/dns/zone.h
View File

@ -2076,19 +2076,20 @@ dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats);
*\li stats is a valid statistics.
*/
isc_stats_t *
dns_zone_getrequeststats(dns_zone_t *zone);
void
dns_zone_getrequeststats(dns_zone_t *zone, isc_stats_t **statsp);
dns_stats_t *
dns_zone_getrcvquerystats(dns_zone_t *zone);
void
dns_zone_getrcvquerystats(dns_zone_t *zone, dns_stats_t **statsp);
dns_stats_t *
dns_zone_getdnssecsignstats(dns_zone_t *zone);
void
dns_zone_getdnssecsignstats(dns_zone_t *zone, dns_stats_t **statsp);
/*%<
* Get the additional statistics for zone, if one is installed.
*
* Requires:
* \li 'zone' to be a valid zone.
* \li 'statsp' to be non-NULL and *stastp to be NULL.
*
* Returns:
* \li when available, a pointer to the statistics set installed in zone;

7
lib/dns/update.c
View File

@ -1107,7 +1107,7 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
dns_kasp_t *kasp = dns_zone_getkasp(zone);
dns_rdataset_t rdataset;
dns_rdata_t sig_rdata = DNS_RDATA_INIT;
dns_stats_t *dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dns_stats_t *dnssecsignstats = NULL;
isc_buffer_t buffer;
unsigned char data[1024]; /* XXX */
unsigned int i;
@ -1121,6 +1121,8 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
offlineksk = dns_kasp_offlineksk(kasp);
}
dns_zone_getdnssecsignstats(zone, &dnssecsignstats);
dns_rdataset_init(&rdataset);
isc_buffer_init(&buffer, data, sizeof(data));
@ -1276,6 +1278,9 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
}
failure:
if (dnssecsignstats != NULL) {
dns_stats_detach(&dnssecsignstats);
}
if (dns_rdataset_isassociated(&rdataset)) {
dns_rdataset_disassociate(&rdataset);
}

64
lib/dns/zone.c
View File

@ -193,6 +193,7 @@ typedef struct dns_include dns_include_t;
} while (0)
#define UNLOCK_ZONE(z) \
do { \
INSIST((z)->locked); \
(z)->locked = false; \
UNLOCK(&(z)->lock); \
} while (0)
@ -6858,7 +6859,7 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
isc_stdtime_t inception, isc_stdtime_t expire) {
isc_result_t result;
dns_dbnode_t *node = NULL;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecsignstats = NULL;
dns_rdataset_t rdataset;
dns_rdata_t sig_rdata = DNS_RDATA_INIT;
unsigned char data[1024]; /* XXX */
@ -7033,7 +7034,7 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
isc_buffer_init(&buffer, data, sizeof(data));
/* Update DNSSEC sign statistics. */
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dns_zone_getdnssecsignstats(zone, &dnssecsignstats);
if (dnssecsignstats != NULL) {
/* Generated a new signature. */
dns_dnssecsignstats_increment(dnssecsignstats,
@ -7045,6 +7046,7 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
dnssecsignstats, ID(keys[i]),
(uint8_t)ALG(keys[i]),
dns_dnssecsignstats_refresh);
dns_stats_detach(&dnssecsignstats);
}
}
@ -7516,7 +7518,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
dns_rdatasetiter_t *iterator = NULL;
dns_rdataset_t rdataset;
dns_rdata_t rdata = DNS_RDATA_INIT;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecsignstats = NULL;
bool offlineksk = false;
isc_buffer_t buffer;
unsigned char data[1024];
@ -7649,7 +7651,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
dns_rdata_reset(&rdata);
/* Update DNSSEC sign statistics. */
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dns_zone_getdnssecsignstats(zone, &dnssecsignstats);
if (dnssecsignstats != NULL) {
/* Generated a new signature. */
dns_dnssecsignstats_increment(dnssecsignstats, ID(key),
@ -7659,6 +7661,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
dns_dnssecsignstats_increment(
dnssecsignstats, ID(key), ALG(key),
dns_dnssecsignstats_refresh);
dns_stats_detach(&dnssecsignstats);
}
(*signatures)--;
@ -19774,15 +19777,27 @@ dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats) {
UNLOCK_ZONE(zone);
}
dns_stats_t *
dns_zone_getdnssecsignstats(dns_zone_t *zone) {
static void
getdnssecsignstats(dns_zone_t *zone, dns_stats_t **statsp) {
REQUIRE(statsp != NULL && *statsp == NULL);
if (zone->dnssecsignstats != NULL) {
dns_stats_attach(zone->dnssecsignstats, statsp);
}
}
void
dns_zone_getdnssecsignstats(dns_zone_t *zone, dns_stats_t **statsp) {
REQUIRE(DNS_ZONE_VALID(zone));
return zone->dnssecsignstats;
LOCK_ZONE(zone);
getdnssecsignstats(zone, statsp);
UNLOCK_ZONE(zone);
}
isc_stats_t *
dns_zone_getrequeststats(dns_zone_t *zone) {
void
dns_zone_getrequeststats(dns_zone_t *zone, isc_stats_t **statsp) {
REQUIRE(DNS_ZONE_VALID(zone));
REQUIRE(statsp != NULL && *statsp == NULL);
/*
* We don't lock zone for efficiency reason. This is not catastrophic
* because requeststats must always be valid when requeststats_on is
@ -19791,24 +19806,27 @@ dns_zone_getrequeststats(dns_zone_t *zone) {
* false, or some cannot be incremented just after the statistics are
* installed, but it shouldn't matter much in practice.
*/
if (zone->requeststats_on) {
return zone->requeststats;
} else {
return NULL;
LOCK_ZONE(zone);
if (zone->requeststats_on && zone->requeststats != NULL) {
isc_stats_attach(zone->requeststats, statsp);
}
UNLOCK_ZONE(zone);
}
/*
* Return the received query stats bucket
* see note from dns_zone_getrequeststats()
*/
dns_stats_t *
dns_zone_getrcvquerystats(dns_zone_t *zone) {
if (zone->requeststats_on) {
return zone->rcvquerystats;
} else {
return NULL;
void
dns_zone_getrcvquerystats(dns_zone_t *zone, dns_stats_t **statsp) {
REQUIRE(DNS_ZONE_VALID(zone));
REQUIRE(statsp != NULL && *statsp == NULL);
LOCK_ZONE(zone);
if (zone->requeststats_on && zone->rcvquerystats != NULL) {
dns_stats_attach(zone->rcvquerystats, statsp);
}
UNLOCK_ZONE(zone);
}
isc_result_t
@ -22617,8 +22635,9 @@ zone_rekey(dns_zone_t *zone) {
if (commit) {
dns_difftuple_t *tuple;
dns_stats_t *dnssecsignstats =
dns_zone_getdnssecsignstats(zone);
dns_stats_t *dnssecsignstats = NULL;
getdnssecsignstats(zone, &dnssecsignstats);
DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDNOTIFY);
@ -22658,6 +22677,9 @@ zone_rekey(dns_zone_t *zone) {
}
}
}
if (dnssecsignstats != NULL) {
dns_stats_detach(&dnssecsignstats);
}
if (fullsign) {
/*

13
lib/ns/query.c
View File

@ -521,7 +521,7 @@ inc_stats(ns_client_t *client, isc_statscounter_t counter) {
dns_zone_t *zone = client->query.authzone;
dns_rdatatype_t qtype;
dns_rdataset_t *rdataset;
isc_stats_t *zonestats;
isc_stats_t *zonestats = NULL;
dns_stats_t *querystats = NULL;
ns_stats_increment(client->manager->sctx->nsstats, counter);
@ -531,10 +531,11 @@ inc_stats(ns_client_t *client, isc_statscounter_t counter) {
}
/* Do regular response type stats */
zonestats = dns_zone_getrequeststats(zone);
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
isc_stats_increment(zonestats, counter);
isc_stats_detach(&zonestats);
}
/* Do query type statistics
@ -543,13 +544,14 @@ inc_stats(ns_client_t *client, isc_statscounter_t counter) {
* answer counter, preventing double-counting.
*/
if (counter == ns_statscounter_authans) {
querystats = dns_zone_getrcvquerystats(zone);
dns_zone_getrcvquerystats(zone, &querystats);
if (querystats != NULL) {
rdataset = ISC_LIST_HEAD(client->query.qname->list);
if (rdataset != NULL) {
qtype = rdataset->type;
dns_rdatatypestats_increment(querystats, qtype);
}
dns_stats_detach(&querystats);
}
}
}
@ -1250,7 +1252,7 @@ rpz_log_rewrite(ns_client_t *client, bool disabled, dns_rpz_policy_t policy,
const char *s1 = cname_buf, *s2 = cname_buf;
dns_rdataset_t *rdataset;
dns_rpz_st_t *st;
isc_stats_t *zonestats;
isc_stats_t *zonestats = NULL;
/*
* Count enabled rewrites in the global counter.
@ -1261,10 +1263,11 @@ rpz_log_rewrite(ns_client_t *client, bool disabled, dns_rpz_policy_t policy,
ns_statscounter_rpz_rewrites);
}
if (p_zone != NULL) {
zonestats = dns_zone_getrequeststats(p_zone);
dns_zone_getrequeststats(p_zone, &zonestats);
if (zonestats != NULL) {
isc_stats_increment(zonestats,
ns_statscounter_rpz_rewrites);
isc_stats_detach(&zonestats);
}
}

4
lib/ns/update.c
View File

@ -329,9 +329,11 @@ inc_stats(ns_client_t *client, dns_zone_t *zone, isc_statscounter_t counter) {
ns_stats_increment(client->manager->sctx->nsstats, counter);
if (zone != NULL) {
isc_stats_t *zonestats = dns_zone_getrequeststats(zone);
isc_stats_t *zonestats = NULL;
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
isc_stats_increment(zonestats, counter);
isc_stats_detach(&zonestats);
}
}
}

4
lib/ns/xfrout.c
View File

@ -111,9 +111,11 @@ static void
inc_stats(ns_client_t *client, dns_zone_t *zone, isc_statscounter_t counter) {
ns_stats_increment(client->manager->sctx->nsstats, counter);
if (zone != NULL) {
isc_stats_t *zonestats = dns_zone_getrequeststats(zone);
isc_stats_t *zonestats = NULL;
dns_zone_getrequeststats(zone, &zonestats);
if (zonestats != NULL) {
isc_stats_increment(zonestats, counter);
isc_stats_detach(&zonestats);
}
}
}