From 30729c7013d0ea2f7eac85f44129df33fb28aaa3 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Thu, 11 Feb 2021 11:32:20 -0300 Subject: [PATCH 1/3] Fix dangling references to outdated views after reconfig This commit fix a leak which was happening every time an inline-signed zone was added to the configuration, followed by a rndc reconfig. During the reconfig process, the secure version of every inline-signed zone was "moved" to a new view upon a reconfig and it "took the raw version along", but only once the secure version was freed (at shutdown) was prev_view for the raw version detached from, causing the old view to be released as well. This caused dangling references to be kept for the previous view, thus keeping all resources used by that view in memory. --- bin/named/server.c | 13 ------------- bin/tests/system/views/ns2/named1.conf.in | 8 ++++++++ lib/dns/zone.c | 6 ++++++ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index e4e80f6f81..c2afd5523a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7975,7 +7975,6 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, isc_result_t result2; dns_view_t *pview = NULL; dns_zone_t *zone = NULL; - dns_zone_t *raw = NULL; zname = cfg_obj_asstring(cfg_tuple_get(zconfig, "name")); origin = dns_fixedname_initname(&fixorigin); @@ -7997,22 +7996,10 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, return; } - dns_zone_getraw(zone, &raw); - if (result == ISC_R_SUCCESS) { dns_zone_setviewcommit(zone); - if (raw != NULL) { - dns_zone_setviewcommit(raw); - } } else { dns_zone_setviewrevert(zone); - if (raw != NULL) { - dns_zone_setviewrevert(raw); - } - } - - if (raw != NULL) { - dns_zone_detach(&raw); } dns_zone_detach(&zone); diff --git a/bin/tests/system/views/ns2/named1.conf.in b/bin/tests/system/views/ns2/named1.conf.in index 4ad0e557e3..64ac6fa8d9 100644 --- a/bin/tests/system/views/ns2/named1.conf.in +++ b/bin/tests/system/views/ns2/named1.conf.in @@ -41,3 +41,11 @@ zone "example" { file "example.db"; allow-update { any; }; }; + +zone "inline" { + type primary; + file "external/inline.db"; + key-directory "external"; + auto-dnssec maintain; + inline-signing yes; +}; diff --git a/lib/dns/zone.c b/lib/dns/zone.c index b0120ec4e3..0269a9b42d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1616,6 +1616,9 @@ dns_zone_setviewcommit(dns_zone_t *zone) { if (zone->prev_view != NULL) { dns_view_weakdetach(&zone->prev_view); } + if (inline_secure(zone)) { + dns_zone_setviewcommit(zone->raw); + } UNLOCK_ZONE(zone); } @@ -1628,6 +1631,9 @@ dns_zone_setviewrevert(dns_zone_t *zone) { dns_zone_setview_helper(zone, zone->prev_view); dns_view_weakdetach(&zone->prev_view); } + if (inline_secure(zone)) { + dns_zone_setviewrevert(zone->raw); + } UNLOCK_ZONE(zone); } From 43b0b20b43d2c7c2d663cb556f013477c50859d3 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Fri, 12 Feb 2021 16:53:34 -0300 Subject: [PATCH 2/3] Test reconfig after adding inline signed zones won't crash named This test ensures that named won't crash after many inline-signed zones are added to configurarion, followed by a rndc reconfig. --- bin/tests/system/views/clean.sh | 2 ++ bin/tests/system/views/ns2/named3.conf.in | 33 ++++++++++++++++++ bin/tests/system/views/tests.sh | 41 +++++++++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 bin/tests/system/views/ns2/named3.conf.in diff --git a/bin/tests/system/views/clean.sh b/bin/tests/system/views/clean.sh index 894b7ce0f7..d4f2e6084c 100644 --- a/bin/tests/system/views/clean.sh +++ b/bin/tests/system/views/clean.sh @@ -27,6 +27,8 @@ rm -f ns2/internal/K* rm -f ns2/internal/inline.db.jbk rm -f ns2/internal/inline.db.signed rm -f ns2/internal/inline.db.signed.jnl +rm -f ns2/zones.conf +rm -f ns2/db.* ns2/K* rm -f dig.out.external dig.out.internal rm -f ns*/named.lock rm -f ns*/managed-keys.bind* ns*/*.mkeys* diff --git a/bin/tests/system/views/ns2/named3.conf.in b/bin/tests/system/views/ns2/named3.conf.in new file mode 100644 index 0000000000..9e36020708 --- /dev/null +++ b/bin/tests/system/views/ns2/named3.conf.in @@ -0,0 +1,33 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * 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 http://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +options { + query-source address 10.53.0.2; + notify-source 10.53.0.2; + transfer-source 10.53.0.2; + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.2; }; + listen-on-v6 { none; }; + recursion no; + notify no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; + +controls { + inet 10.53.0.2 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +include "zones.conf"; diff --git a/bin/tests/system/views/tests.sh b/bin/tests/system/views/tests.sh index 01ea650652..bb721c4c67 100644 --- a/bin/tests/system/views/tests.sh +++ b/bin/tests/system/views/tests.sh @@ -132,5 +132,46 @@ test "$int" != "$ext" || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +echo_i "verifying adding of multiple inline zones followed by reconfiguration works" + +[ ! -f ns2/zones.conf ] && touch ns2/zones.conf +copy_setports ns2/named3.conf.in ns2/named.conf + +for i in `seq 1 50`; do + ret=0 + zone_name=`printf "example%03d.com" $i` + +# Add a new zone to the configuration. + cat >> ns2/zones.conf << EOF +zone "${zone_name}" { + type master; + file "db.${zone_name}"; + dnssec-dnskey-kskonly yes; + auto-dnssec maintain; + inline-signing yes; +}; +EOF + +# Create a master file for the zone. + cat > "ns2/db.${zone_name}" < /dev/null + $RNDCCMD 10.53.0.2 reconfig || ret=1 + if [ $ret != 0 ]; then echo_i "failed"; break; fi +done # end for # +status=`expr $status + $ret` + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From 757be6ec1604d8ba2c26d665d8270d7d354b1237 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Thu, 11 Feb 2021 12:22:00 -0300 Subject: [PATCH 3/3] Add CHANGES note for [GL #2041] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 6f335a8187..c5f893bd90 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5581. [bug] Fix memory leak happening when inline-signed zones + were added to the configuration followed by a + reconfiguration of named. [GL #2041] + 5580. [test] The system test framework no longer differentiates between SKIPPED and UNTESTED system test results. Any system test which is not run is now marked as SKIPPED.