From 65a3694f2ab7a9ce4fb61ead3d0d722f9f2bae38 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 13 May 2026 14:37:42 -0700 Subject: [PATCH 1/2] Check dns_rdata_fromstruct() return values The in some functions implementing RFC 5011 key maintenance, the results of dns_rdata_fromstruct() were not checked. This has been fixed. (cherry picked from commit 175d418e2840b2779b4aa21ae7663eddab5cf1d9) --- lib/dns/zone.c | 79 +++++++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 8d8b3d12bd..e3e93379e1 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -4269,15 +4269,15 @@ compute_tag(dns_name_t *name, dns_rdata_dnskey_t *dnskey, isc_mem_t *mctx, dst_key_t *dstkey = NULL; isc_buffer_init(&buffer, data, sizeof(data)); - dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, - dns_rdatatype_dnskey, dnskey, &buffer); - result = dns_dnssec_keyfromrdata(name, &rdata, mctx, &dstkey); - if (result == ISC_R_SUCCESS) { - *tag = dst_key_id(dstkey); - dst_key_free(&dstkey); - } + CHECK(dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, + dns_rdatatype_dnskey, dnskey, &buffer)); + CHECK(dns_dnssec_keyfromrdata(name, &rdata, mctx, &dstkey)); + *tag = dst_key_id(dstkey); + dst_key_free(&dstkey); + +cleanup: return result; } @@ -4311,15 +4311,12 @@ trust_key(dns_zone_t *zone, dns_name_t *keyname, dns_rdata_dnskey_t *dnskey, dns_keytable_t *sr = NULL; dns_rdata_ds_t ds; - result = dns_view_getsecroots(zone->view, &sr); - if (result != ISC_R_SUCCESS) { - return; - } + CHECK(dns_view_getsecroots(zone->view, &sr)); /* Build DS record for key. */ isc_buffer_init(&buffer, data, sizeof(data)); - dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, - dns_rdatatype_dnskey, dnskey, &buffer); + CHECK(dns_rdata_fromstruct(&rdata, dnskey->common.rdclass, + dns_rdatatype_dnskey, dnskey, &buffer)); CHECK(dns_ds_fromkeyrdata(keyname, &rdata, DNS_DSDIGEST_SHA256, digest, &ds)); CHECK(dns_keytable_add(sr, true, initial, keyname, &ds, sfd_add, @@ -10034,7 +10031,7 @@ normalize_key(dns_rdata_t *rr, dns_rdata_t *target, unsigned char *data, dns_rdata_dnskey_t dnskey; dns_rdata_keydata_t keydata; isc_buffer_t buf; - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; dns_rdata_reset(target); isc_buffer_init(&buf, data, size); @@ -10044,8 +10041,9 @@ normalize_key(dns_rdata_t *rr, dns_rdata_t *target, unsigned char *data, result = dns_rdata_tostruct(rr, &dnskey, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); dnskey.flags &= ~DNS_KEYFLAG_REVOKE; - dns_rdata_fromstruct(target, rr->rdclass, dns_rdatatype_dnskey, - &dnskey, &buf); + result = dns_rdata_fromstruct(target, rr->rdclass, + dns_rdatatype_dnskey, &dnskey, + &buf); break; case dns_rdatatype_keydata: result = dns_rdata_tostruct(rr, &keydata, NULL); @@ -10054,13 +10052,15 @@ normalize_key(dns_rdata_t *rr, dns_rdata_t *target, unsigned char *data, } RUNTIME_CHECK(result == ISC_R_SUCCESS); dns_keydata_todnskey(&keydata, &dnskey, NULL); - dns_rdata_fromstruct(target, rr->rdclass, dns_rdatatype_dnskey, - &dnskey, &buf); + result = dns_rdata_fromstruct(target, rr->rdclass, + dns_rdatatype_dnskey, &dnskey, + &buf); break; default: UNREACHABLE(); } - return ISC_R_SUCCESS; + + return result; } /* @@ -10260,8 +10260,13 @@ revocable(dns_keyfetch_t *kfetch, dns_rdata_keydata_t *keydata) { /* Generate a key from keydata */ isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); dns_keydata_todnskey(keydata, &dnskey, NULL); - dns_rdata_fromstruct(&rr, keydata->common.rdclass, dns_rdatatype_dnskey, - &dnskey, &keyb); + + result = dns_rdata_fromstruct(&rr, keydata->common.rdclass, + dns_rdatatype_dnskey, &dnskey, &keyb); + if (result != ISC_R_SUCCESS) { + return false; + } + result = dns_dnssec_keyfromrdata(keyname, &rr, mctx, &dstkey); if (result != ISC_R_SUCCESS) { return false; @@ -10615,9 +10620,16 @@ anchors_done: dns_rdata_reset(&keydatarr); isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); - dns_rdata_fromstruct(&keydatarr, zone->rdclass, - dns_rdatatype_keydata, &keydata, - &keyb); + result = dns_rdata_fromstruct(&keydatarr, zone->rdclass, + dns_rdatatype_keydata, + &keydata, &keyb); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_WARNING, + "dns_rdata_fromstruct failed: " + "KEYDATA %d: %s", + keytag, isc_result_totext(result)); + continue; + } /* Insert updated version */ CHECK(update_one_rr(kfetch->db, ver, &diff, @@ -10846,9 +10858,16 @@ anchors_done: keydata.refresh = refresh_time(kfetch, false); dns_rdata_reset(&keydatarr); isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); - dns_rdata_fromstruct(&keydatarr, zone->rdclass, - dns_rdatatype_keydata, &keydata, - &keyb); + result = dns_rdata_fromstruct(&keydatarr, zone->rdclass, + dns_rdatatype_keydata, + &keydata, &keyb); + if (result != ISC_R_SUCCESS) { + dnssec_log(zone, ISC_LOG_WARNING, + "dns_rdata_fromstruct failed: " + "KEYDATA %d: %s", + keytag, isc_result_totext(result)); + continue; + } /* Insert updated version */ CHECK(update_one_rr(kfetch->db, ver, &diff, @@ -10866,9 +10885,9 @@ anchors_done: keydata.refresh = refresh_time(kfetch, false); dns_rdata_reset(&keydatarr); isc_buffer_init(&keyb, key_buf, sizeof(key_buf)); - dns_rdata_fromstruct(&keydatarr, zone->rdclass, - dns_rdatatype_keydata, &keydata, - &keyb); + CHECK(dns_rdata_fromstruct(&keydatarr, zone->rdclass, + dns_rdatatype_keydata, + &keydata, &keyb)); /* Insert into key zone */ CHECK(update_one_rr(kfetch->db, ver, &diff, From 849aaacaae5613f959ddca2e6d3d36428945bc77 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 4 Jun 2026 09:49:41 -0700 Subject: [PATCH 2/2] Add "warn_unused_result" to attributes.h A new ISC_ATTR_WARN_UNUSED_RESULT macro now defines __attribute__((warn_unused_result)) and is used for dns_rdata_fromstruct(). (cherry picked from commit 07c8c1d2421fdb4beb1b7821fe9033ff7d66b46a) --- configure.ac | 5 +++++ lib/dns/include/dns/rdata.h | 1 + lib/isc/include/isc/attributes.h | 6 ++++++ tests/dns/private_test.c | 6 ++++-- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 53c3f26976..2278a5d763 100644 --- a/configure.ac +++ b/configure.ac @@ -491,6 +491,11 @@ AC_COMPILE_IFELSE( # AX_GCC_FUNC_ATTRIBUTE([returns_nonnull]) +# +# check for GCC returns_nonnull attribute +# +AX_GCC_FUNC_ATTRIBUTE([warn_unused_result]) + # # how to link math functions? # diff --git a/lib/dns/include/dns/rdata.h b/lib/dns/include/dns/rdata.h index 02ac4eef22..b014c5751f 100644 --- a/lib/dns/include/dns/rdata.h +++ b/lib/dns/include/dns/rdata.h @@ -468,6 +468,7 @@ dns_rdata_tofmttext(dns_rdata_t *rdata, const dns_name_t *origin, * it is undefined and falls back to the default value of 'width' */ +ISC_ATTR_WARN_UNUSED_RESULT isc_result_t dns_rdata_fromstruct(dns_rdata_t *rdata, dns_rdataclass_t rdclass, dns_rdatatype_t type, void *source, isc_buffer_t *target); diff --git a/lib/isc/include/isc/attributes.h b/lib/isc/include/isc/attributes.h index aab0629c45..15958b93a9 100644 --- a/lib/isc/include/isc/attributes.h +++ b/lib/isc/include/isc/attributes.h @@ -35,6 +35,12 @@ #define ISC_ATTR_RETURNS_NONNULL #endif +#if HAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT +#define ISC_ATTR_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) +#else +#define ISC_ATTR_WARN_UNUSED_RESULT +#endif + #ifdef HAVE_FUNC_ATTRIBUTE_MALLOC /* * Indicates that a function is malloc-like, i.e., that the diff --git a/tests/dns/private_test.c b/tests/dns/private_test.c index f65408bdd0..0ab974f34c 100644 --- a/tests/dns/private_test.c +++ b/tests/dns/private_test.c @@ -97,6 +97,7 @@ make_signing(signing_testcase_t *testcase, dns_rdata_t *private, static void make_nsec3(nsec3_testcase_t *testcase, dns_rdata_t *private, unsigned char *pbuf) { + isc_result_t result; dns_rdata_nsec3param_t params; dns_rdata_t nsec3param = DNS_RDATA_INIT; unsigned char bufdata[BUFSIZ]; @@ -134,8 +135,9 @@ make_nsec3(nsec3_testcase_t *testcase, dns_rdata_t *private, } isc_buffer_init(&buf, bufdata, sizeof(bufdata)); - dns_rdata_fromstruct(&nsec3param, dns_rdataclass_in, - dns_rdatatype_nsec3param, ¶ms, &buf); + result = dns_rdata_fromstruct(&nsec3param, dns_rdataclass_in, + dns_rdatatype_nsec3param, ¶ms, &buf); + assert_int_equal(result, ISC_R_SUCCESS); dns_rdata_init(private);