From: Ondřej Surý Date: Sun, 2 Feb 2025 12:31:36 +0000 (+0100) Subject: In dns_slabheader_t structure, change .ttl to .expire X-Git-Tag: v9.21.5~9^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e07f5a4a5bb3aac984743bfba409da47ccc17e7d;p=thirdparty%2Fbind9.git In dns_slabheader_t structure, change .ttl to .expire The old name was misleading as it never meant time-to-live, e.g. number of seconds from now when the header should expire. The true meaning was an expiration time e.g. now + ttl. This was the original design bug that caused the slip when we assigned header->ttl to rdataset->ttl. Because the name was matching, nobody has questioned the correctness of the code both during the MR review and during the numerous re-reviews when we were searching for the cause of the 54 year TTL. --- diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h index 82660b5d9d6..ff2057808f4 100644 --- a/lib/dns/include/dns/rdataslab.h +++ b/lib/dns/include/dns/rdataslab.h @@ -72,7 +72,7 @@ struct dns_slabheader { */ dns_trust_t trust; uint32_t serial; - dns_ttl_t ttl; + isc_stdtime_t expire; dns_typepair_t type; _Atomic(uint16_t) count; diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 566490bc710..589ddd0aa11 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -108,8 +108,9 @@ #define STALE_TTL(header, qpdb) \ (NXDOMAIN(header) ? 0 : qpdb->common.serve_stale_ttl) -#define ACTIVE(header, now) \ - (((header)->ttl > (now)) || ((header)->ttl == (now) && ZEROTTL(header))) +#define ACTIVE(header, now) \ + (((header)->expire > (now)) || \ + ((header)->expire == (now) && ZEROTTL(header))) #define EXPIREDOK(iterator) \ (((iterator)->common.options & DNS_DB_EXPIREDOK) != 0) @@ -925,10 +926,10 @@ mark(dns_slabheader_t *header, uint_least16_t flag) { } static void -setttl(dns_slabheader_t *header, dns_ttl_t newttl) { - dns_ttl_t oldttl = header->ttl; +setttl(dns_slabheader_t *header, isc_stdtime_t newts) { + isc_stdtime_t oldts = header->expire; - header->ttl = newttl; + header->expire = newts; if (header->db == NULL || !dns_db_iscache(header->db)) { return; @@ -937,18 +938,17 @@ setttl(dns_slabheader_t *header, dns_ttl_t newttl) { /* * This is a cache. Adjust the heaps if necessary. */ - if (header->heap == NULL || header->heap_index == 0 || newttl == oldttl) - { + if (header->heap == NULL || header->heap_index == 0 || newts == oldts) { return; } - if (newttl < oldttl) { + if (newts < oldts) { isc_heap_increased(header->heap, header->heap_index); } else { isc_heap_decreased(header->heap, header->heap_index); } - if (newttl == 0) { + if (newts == 0) { isc_heap_delete(header->heap, header->heap_index); } } @@ -1049,7 +1049,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, * Mark header stale or ancient if the RRset is no longer active. */ if (!ACTIVE(header, now)) { - dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb); + dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb); /* * If this data is in the stale window keep it and if * DNS_DBFIND_STALEOK is not set we tell the caller to @@ -1072,7 +1072,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, rdataset->rdclass = qpdb->common.rdclass; rdataset->type = DNS_TYPEPAIR_TYPE(header->type); rdataset->covers = DNS_TYPEPAIR_COVERS(header->type); - rdataset->ttl = header->ttl - now; + rdataset->ttl = header->expire - now; rdataset->trust = header->trust; rdataset->resign = 0; @@ -1090,7 +1090,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, } if (stale && !ancient) { - dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb); + dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb); if (stale_ttl > now) { rdataset->ttl = stale_ttl - now; } else { @@ -1180,7 +1180,8 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header, isc_rwlocktype_t *nlocktypep, isc_rwlock_t *nlock, qpc_search_t *search, dns_slabheader_t **header_prev) { if (!ACTIVE(header, search->now)) { - dns_ttl_t stale = header->ttl + STALE_TTL(header, search->qpdb); + dns_ttl_t stale = header->expire + + STALE_TTL(header, search->qpdb); /* * If this data is in the stale window keep it and if * DNS_DBFIND_STALEOK is not set we tell the caller to @@ -1239,7 +1240,7 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header, * it as ancient, and the node as dirty, so it will get * cleaned up later. */ - if ((header->ttl < search->now - QPDB_VIRTUAL) && + if ((header->expire < search->now - QPDB_VIRTUAL) && (*nlocktypep == isc_rwlocktype_write || NODE_TRYUPGRADE(nlock, nlocktypep) == ISC_R_SUCCESS)) { @@ -2218,7 +2219,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, for (header = qpnode->data; header != NULL; header = header_next) { header_next = header->next; if (!ACTIVE(header, now)) { - if ((header->ttl + STALE_TTL(header, qpdb) < + if ((header->expire + STALE_TTL(header, qpdb) < now - QPDB_VIRTUAL) && (nlocktype == isc_rwlocktype_write || NODE_TRYUPGRADE(nlock, &nlocktype) == @@ -2476,7 +2477,7 @@ ttl_sooner(void *v1, void *v2) { dns_slabheader_t *h1 = v1; dns_slabheader_t *h2 = v2; - return h1->ttl < h2->ttl; + return h1->expire < h2->expire; } /*% @@ -3019,8 +3020,8 @@ find_header: * Honour the new ttl if it is less than the * older one. */ - if (header->ttl > newheader->ttl) { - setttl(header, newheader->ttl); + if (header->expire > newheader->expire) { + setttl(header, newheader->expire); } if (header->last_used != now) { ISC_LIST_UNLINK( @@ -3061,8 +3062,8 @@ find_header: !header_nx && !newheader_nx && header->trust <= newheader->trust) { - if (newheader->ttl > header->ttl) { - newheader->ttl = header->ttl; + if (newheader->expire > header->expire) { + newheader->expire = header->expire; } } if (ACTIVE(header, now) && @@ -3081,8 +3082,8 @@ find_header: * Honour the new ttl if it is less than the * older one. */ - if (header->ttl > newheader->ttl) { - setttl(header, newheader->ttl); + if (header->expire > newheader->expire) { + setttl(header, newheader->expire); } if (header->last_used != now) { ISC_LIST_UNLINK( @@ -3726,7 +3727,7 @@ rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp DNS__DB_FLARG) { static bool iterator_active(qpcache_t *qpdb, qpc_rditer_t *iterator, dns_slabheader_t *header) { - dns_ttl_t stale_ttl = header->ttl + STALE_TTL(header, qpdb); + dns_ttl_t stale_ttl = header->expire + STALE_TTL(header, qpdb); /* * Is this a "this rdataset doesn't exist" record? @@ -4283,7 +4284,7 @@ expire_ttl_headers(qpcache_t *qpdb, unsigned int locknum, return; } - dns_ttl_t ttl = header->ttl; + dns_ttl_t ttl = header->expire; if (!cache_is_overmem) { /* Only account for stale TTL if cache is not overmem */ diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 96282b78522..9771163fdc7 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -985,7 +985,7 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header, rdataset->rdclass = qpdb->common.rdclass; rdataset->type = DNS_TYPEPAIR_TYPE(header->type); rdataset->covers = DNS_TYPEPAIR_COVERS(header->type); - rdataset->ttl = header->ttl - now; + rdataset->ttl = header->expire - now; rdataset->trust = header->trust; if (OPTOUT(header)) { @@ -1890,10 +1890,10 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename, flags |= DNS_RDATASLAB_EXACT; } if ((options & DNS_DBADD_EXACTTTL) != 0 && - newheader->ttl != header->ttl) + newheader->expire != header->expire) { result = DNS_R_NOTEXACT; - } else if (newheader->ttl != header->ttl) { + } else if (newheader->expire != header->expire) { flags |= DNS_RDATASLAB_FORCE; } if (result == ISC_R_SUCCESS) { @@ -2181,7 +2181,7 @@ loading_addrdataset(void *arg, const dns_name_t *name, newheader = (dns_slabheader_t *)region.base; *newheader = (dns_slabheader_t){ .type = DNS_TYPEPAIR_VALUE(rdataset->type, rdataset->covers), - .ttl = rdataset->ttl + loadctx->now, + .expire = rdataset->ttl + loadctx->now, .trust = rdataset->trust, .node = (dns_dbnode_t *)node, .serial = 1, @@ -4687,7 +4687,7 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, }; dns_slabheader_reset(newheader, db, (dns_dbnode_t *)node); - newheader->ttl = rdataset->ttl; + newheader->expire = rdataset->ttl; if (rdataset->ttl == 0U) { DNS_SLABHEADER_SETATTR(newheader, DNS_SLABHEADERATTR_ZEROTTL); } @@ -4802,7 +4802,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, newheader = (dns_slabheader_t *)region.base; dns_slabheader_reset(newheader, db, (dns_dbnode_t *)node); - newheader->ttl = rdataset->ttl; + newheader->expire = rdataset->ttl; newheader->type = DNS_TYPEPAIR_VALUE(rdataset->type, rdataset->covers); atomic_init(&newheader->attributes, 0); newheader->serial = version->serial; @@ -4852,7 +4852,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, result = ISC_R_SUCCESS; if ((options & DNS_DBSUB_EXACT) != 0) { flags |= DNS_RDATASLAB_EXACT; - if (newheader->ttl != header->ttl) { + if (newheader->expire != header->expire) { result = DNS_R_NOTEXACT; } } @@ -4899,7 +4899,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, dns_slabheader_destroy(&newheader); newheader = dns_slabheader_new((dns_db_t *)qpdb, (dns_dbnode_t *)node); - newheader->ttl = 0; + newheader->expire = 0; newheader->type = topheader->type; atomic_init(&newheader->attributes, DNS_SLABHEADERATTR_NONEXISTENT); @@ -4983,7 +4983,7 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode, newheader = dns_slabheader_new(db, (dns_dbnode_t *)node); newheader->type = DNS_TYPEPAIR_VALUE(type, covers); - newheader->ttl = 0; + newheader->expire = 0; atomic_init(&newheader->attributes, DNS_SLABHEADERATTR_NONEXISTENT); newheader->serial = version->serial;