From 37f265f59aabefec9d2b4721be083bad466e04c8 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 7 May 2026 11:44:44 +0000 Subject: [PATCH 1/6] Add a new check in "inline" system test This new check floods the server with DNS UPDATE messages for an 'inline-signing yes; sig-signing-signatures 1;' zone to see if it manages to process the updates correctly. --- bin/tests/system/inline/ns3/named.conf.j2 | 9 +++ bin/tests/system/inline/setup.sh | 1 + .../tests_inline_incremental_updates.py | 65 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 bin/tests/system/inline/tests_inline_incremental_updates.py diff --git a/bin/tests/system/inline/ns3/named.conf.j2 b/bin/tests/system/inline/ns3/named.conf.j2 index 55e2048117..fd28e9da0b 100644 --- a/bin/tests/system/inline/ns3/named.conf.j2 +++ b/bin/tests/system/inline/ns3/named.conf.j2 @@ -57,6 +57,15 @@ zone "bits" { sig-signing-signatures 1; // force incremental processing }; +zone "incremental-updates" { + type primary; + file "incremental-updates.db"; + inline-signing yes; + dnssec-policy inline; + sig-signing-signatures 1; // force incremental processing + allow-update { any; }; +}; + server 10.53.0.4 { request-ixfr no; }; zone "noixfr" { diff --git a/bin/tests/system/inline/setup.sh b/bin/tests/system/inline/setup.sh index c1b38e8606..50d2a07347 100644 --- a/bin/tests/system/inline/setup.sh +++ b/bin/tests/system/inline/setup.sh @@ -31,6 +31,7 @@ cp ns3/primary.db.in ns3/nsec3.db cp ns3/primary.db.in ns3/externalkey.db cp ns3/primary.db.in ns3/delayedkeys.db cp ns3/primary.db.in ns3/removedkeys-primary.db +cp ns3/primary.db.in ns3/incremental-updates.db cp ns3/include.db.in ns3/include.db mkdir ns3/removedkeys diff --git a/bin/tests/system/inline/tests_inline_incremental_updates.py b/bin/tests/system/inline/tests_inline_incremental_updates.py new file mode 100644 index 0000000000..fdc3c18d63 --- /dev/null +++ b/bin/tests/system/inline/tests_inline_incremental_updates.py @@ -0,0 +1,65 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import random +import threading + +import dns.update +import pytest + +pytestmark = pytest.mark.extra_artifacts( + [ + "K*", + "*.out*", + "*/*.out*", + "ns*/K*", + "ns*/dsset-*", + "ns*/*.bk", + "ns*/*.db", + "ns*/*.jbk", + "ns*/*.jnl", + "ns*/*.nzd", + "ns*/*.signed", + "ns*/trusted.conf", + "ns3/delayedkeys.conf", + "ns3/removedkeys", + ] +) + + +def worker(server, count) -> None: + zone = "incremental-updates" + for i in range(count): + try: + sub = random.randrange(1000000) + update_msg = dns.update.UpdateMessage(zone) + update_msg.add(f"a-{sub}-{i}.{zone}.", 300, "A", "10.0.0.1") + server.nsupdate(update_msg) + except Exception: # pylint: disable=broad-exception-caught + break + + +def test_inline_incremental_updates(ns3): + """ + Flood the server with updates to check how 'receive secure serial' + is coping with quick incremental updates. + """ + threads_n = 10 + updates_n = 10 + threads = [ + threading.Thread(target=worker, args=(ns3, updates_n), daemon=True) + for _ in range(threads_n) + ] + + for thread in threads: + thread.start() + for thread in threads: + thread.join() From 7d7130f51a2895c4fbb56a64bb1ecde40d81a700 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 3 Apr 2026 15:16:10 +0000 Subject: [PATCH 2/6] Remove redundant INSIST The check is already performed in the INSIST above. --- lib/dns/zone.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index d6a597abfd..f23b4cba6d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14400,7 +14400,6 @@ receive_secure_serial(void *arg) { INSIST(zone->rss == NULL || zone->rss == rss); if (zone->rss != NULL) { - INSIST(zone->rss == rss); UNLOCK_ZONE(zone); } else { zone->rss = rss; From 5118f3bad62ade03b1a2c345c7dbfa9629ad9c7f Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 3 Apr 2026 15:55:55 +0000 Subject: [PATCH 3/6] Fix a bug in "receive secure serial" processing When a DNS UPDATE messages is received, the zone_send_secureserial() function can schedule a new receive_secure_serial() call with a new 'rss' object before a previous one had a chance to be fully processed. This can cause an assertion failure in receive_secure_serial() with a new 'rss' object (when the old one was rescheduled because it got DNS_R_CONTINUE from dns_update_signaturesinc()). In other words: 1. receive_secure_serial() called with rss, sets zone->rss = rss, reschedules because of DNS_R_CONTINUE 2. receive_secure_serial() called with rss_new, INSIST fails because zone->rss != rss_new), i.e. this was called before the old 'rss' was fully processed Change the code logic by introducing a new 'rss_next' field and making sure the old 'rss' is complete before starting processing the new one. --- lib/dns/zone.c | 52 +++++++++++++++++++++++++++++++++++------------- lib/dns/zone_p.h | 1 + 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index f23b4cba6d..3655661d06 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14393,16 +14393,29 @@ receive_secure_serial(void *arg) { LOCK_ZONE(zone); /* - * The receive_secure_serial() is loop-serialized for the zone. Make - * sure there's no processing currently running. + * The receive_secure_serial() is loop-serialized for the zone, but it + * is possible for new serial to arrive before the old processing is + * completed, because the old call could have been rescheduled if + * dns_update_signaturesinc() below returned DNS_R_CONTINUE. */ - - INSIST(zone->rss == NULL || zone->rss == rss); - if (zone->rss != NULL) { + /* There is an existing processing */ UNLOCK_ZONE(zone); + if (zone->rss != rss) { + /* + * A new 'rss' is sandwiched between a previous call + * with the old 'zone->rss' and a future scheduled + * call with the old 'zone->rss'. Reschedule the new + * 'rss' so the old one can be completed first. + */ + isc_async_run(zone->loop, receive_secure_serial, rss); + return; + } } else { - zone->rss = rss; + /* New arrival */ + INSIST(rss == zone->rss_next); + zone->rss = MOVE_OWNERSHIP(zone->rss_next); + dns_diff_init(zone->mctx, &zone->rss_diff); /* @@ -14518,6 +14531,7 @@ receive_secure_serial(void *arg) { &log, zone, zone->rss_db, zone->rss_oldver, zone->rss_newver, &zone->rss_diff, zone->sigvalidityinterval, &zone->rss_state); if (result == DNS_R_CONTINUE) { + /* Signing quantum reached, reschedule the next chunk. */ if (rjournal != NULL) { dns_journal_destroy(&rjournal); } @@ -14622,15 +14636,25 @@ static isc_result_t zone_send_secureserial(dns_zone_t *zone, uint32_t serial) { struct rss *rss = NULL; - rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); - *rss = (struct rss){ - .serial = serial, - .link = ISC_LINK_INITIALIZER, - }; - INSIST(LOCKED_ZONE(zone->secure)); - zone_iattach(zone->secure, &rss->zone); - isc_async_run(zone->secure->loop, receive_secure_serial, rss); + + if (zone->secure->rss_next != NULL) { + /* + * New version came earlier than the previous one had a + * chance to be processed, just update the serial. + */ + zone->secure->rss_next->serial = serial; + } else { + rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); + *rss = (struct rss){ + .serial = serial, + .link = ISC_LINK_INITIALIZER, + }; + zone_iattach(zone->secure, &rss->zone); + + zone->secure->rss_next = rss; + isc_async_run(zone->secure->loop, receive_secure_serial, rss); + } DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_SENDSECURE); return ISC_R_SUCCESS; diff --git a/lib/dns/zone_p.h b/lib/dns/zone_p.h index f7428c3a24..4e40944358 100644 --- a/lib/dns/zone_p.h +++ b/lib/dns/zone_p.h @@ -582,6 +582,7 @@ struct dns_zone { dns_db_t *rss_db; dns_zone_t *rss_raw; struct rss *rss; + struct rss *rss_next; dns_update_state_t *rss_state; isc_stats_t *gluecachestats; From fad2c5b757dddb6b973c82a8915bc8d8d659ca5a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 3 Apr 2026 15:57:26 +0000 Subject: [PATCH 4/6] Remove the unused link from 'struct rss' The link is declared but never used. Remove it. --- lib/dns/zone.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 3655661d06..5b1ff55af4 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -13799,7 +13799,6 @@ struct rss { dns_zone_t *zone; dns_db_t *db; uint32_t serial; - ISC_LINK(struct rss) link; }; static void @@ -14648,7 +14647,6 @@ zone_send_secureserial(dns_zone_t *zone, uint32_t serial) { rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); *rss = (struct rss){ .serial = serial, - .link = ISC_LINK_INITIALIZER, }; zone_iattach(zone->secure, &rss->zone); @@ -15080,7 +15078,7 @@ zone_send_securedb(dns_zone_t *zone, dns_db_t *db) { struct rss *rss = NULL; rss = isc_mem_get(zone->secure->mctx, sizeof(*rss)); - *rss = (struct rss){ .link = ISC_LINK_INITIALIZER }; + *rss = (struct rss){ 0 }; INSIST(LOCKED_ZONE(zone->secure)); zone_iattach(zone->secure, &rss->zone); From b56e3cd2ee1489b1482a6706d8e6073ac96b2625 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 7 May 2026 13:31:13 +0000 Subject: [PATCH 5/6] Handle zone shutting down case in receive_secure_serial() When the zone is shutting down jump straight to cleanup, otherwise an assertion failure is possible, e.g. because zone->loop can be already NULL. --- lib/dns/zone.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 5b1ff55af4..bfb3eec6e1 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14391,6 +14391,27 @@ receive_secure_serial(void *arg) { LOCK_ZONE(zone); + if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING) || + !dns__zone_inline_secure(zone)) + { + /* + * If this is a callback for a new secure serial that was + * never processed and the zone is shutting down, then just + * free 'rss_next' and return. + */ + if (rss == zone->rss_next) { + isc_mem_put(zone->mctx, rss, sizeof(*rss)); + zone->rss_next = NULL; + UNLOCK_ZONE(zone); + dns_zone_idetach(&zone); + return; + } + + /* Otherwise, this is an ongoing processing, do the cleanup. */ + UNLOCK_ZONE(zone); + CLEANUP(ISC_R_SHUTTINGDOWN); + } + /* * The receive_secure_serial() is loop-serialized for the zone, but it * is possible for new serial to arrive before the old processing is @@ -14592,7 +14613,7 @@ cleanup: if (zone->rss_raw != NULL) { dns_zone_detach(&zone->rss_raw); } - if (result != ISC_R_SUCCESS) { + if (result != ISC_R_SUCCESS && result != ISC_R_SHUTTINGDOWN) { LOCK_ZONE(zone); dns__zone_set_resigntime(zone); timenow = isc_time_now(); From 5e129066691c42a15e0864cc0f1f393781226abf Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 7 May 2026 23:24:22 +0000 Subject: [PATCH 6/6] Fix memory leak bug during zone shutdown The dns_update_signaturesinc() updates zone signatures in chunks, keeping its current state in 'zone->rss_state'. When a zone shuts down, the signature update process is canceled, and all the data in the state is not freed. Create a new dns_update_state_clear() function which can be called from dns_zone_free() to free the memory. --- lib/dns/include/dns/update.h | 5 ++++- lib/dns/update.c | 28 ++++++++++++++++++++-------- lib/dns/zone.c | 4 ++++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/dns/include/dns/update.h b/lib/dns/include/dns/update.h index ec406dca0c..b0fdf17fb5 100644 --- a/lib/dns/include/dns/update.h +++ b/lib/dns/include/dns/update.h @@ -63,4 +63,7 @@ isc_result_t dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *oldver, dns_dbversion_t *newver, dns_diff_t *diff, uint32_t sigvalidityinterval, - dns_update_state_t **state); + dns_update_state_t **statep); + +void +dns_update_state_clear(dns_update_state_t **statep, bool destroy); diff --git a/lib/dns/update.c b/lib/dns/update.c index e1cb886713..276a06f762 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1310,6 +1310,7 @@ dns_update_signatures(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, struct dns_update_state { unsigned int magic; + isc_mem_t *mctx; dns_diff_t diffnames; dns_diff_t affected; dns_diff_t sig_diff; @@ -1366,10 +1367,9 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, dns_diff_t *diff, uint32_t sigvalidityinterval, dns_update_state_t **statep) { isc_result_t result = ISC_R_SUCCESS; - dns_update_state_t mystate, *state = NULL; + dns_update_state_t mystate = { 0 }, *state = NULL; dns_difftuple_t *tuple = NULL; bool flag, build_nsec; - unsigned int i; dns_rdata_soa_t soa; dns_rdata_t rdata = DNS_RDATA_INIT; dns_rdataset_t rdataset; @@ -1385,7 +1385,10 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, state = &mystate; } else { state = isc_mem_get(diff->mctx, sizeof(*state)); + state->mctx = NULL; + isc_mem_attach(diff->mctx, &state->mctx); } + state->magic = STATE_MAGIC; dns_diff_init(diff->mctx, &state->diffnames); dns_diff_init(diff->mctx, &state->affected); @@ -1444,7 +1447,6 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, */ CHECK(dns_diff_sort(diff, temp_order)); state->state = sign_updates; - state->magic = STATE_MAGIC; SET_IF_NOT_NULL(statep, state); } else { REQUIRE(DNS_STATE_VALID(*statep)); @@ -2005,6 +2007,18 @@ cleanup: dns_db_detachnode(&node); } + dns_update_state_clear(&state, state != &mystate); + SET_IF_NOT_NULL(statep, NULL); + + return result; +} + +void +dns_update_state_clear(dns_update_state_t **statep, bool destroy) { + REQUIRE(DNS_STATE_VALID(*statep)); + + dns_update_state_t *state = *statep; + dns_diff_clear(&state->sig_diff); dns_diff_clear(&state->nsec_diff); dns_diff_clear(&state->nsec_mindiff); @@ -2013,17 +2027,15 @@ cleanup: dns_diff_clear(&state->diffnames); dns_diff_clear(&state->work); - for (i = 0; i < state->nkeys; i++) { + for (size_t i = 0; i < state->nkeys; i++) { dst_key_free(&state->zone_keys[i]); } - if (state != &mystate) { + if (destroy) { *statep = NULL; state->magic = 0; - isc_mem_put(diff->mctx, state, sizeof(*state)); + isc_mem_putanddetach(&state->mctx, state, sizeof(*state)); } - - return result; } static isc_stdtime_t diff --git a/lib/dns/zone.c b/lib/dns/zone.c index bfb3eec6e1..d50cbbaf34 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -593,6 +593,10 @@ dns__zone_free(dns_zone_t *zone) { isc_mem_put(zone->mctx, include, sizeof *include); } + if (zone->rss_state != NULL) { + dns_update_state_clear(&zone->rss_state, true); + } + if (zone->masterfile != NULL) { isc_mem_free(zone->mctx, zone->masterfile); }