From efcba4dd23e1c63105c15d87aab09c695e3bfd11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 20 Dec 2023 17:21:14 +0100 Subject: [PATCH 1/2] Do not destroy IXFR journal in xfrin_end() The xfrin_end() function is run when a zone transfer is finished or canceled. One of the actions it takes for incremental transfers (IXFR) is calling dns_journal_destroy() on the zone journal structure that is stored in the relevant zone transfer context (xfr->ixfr.journal). That immediately invalidates that structure as it is not reference-counted. However, since the changes present in the IXFR stream are applied to the journal asynchronously (via isc_work_enqueue()), it is possible that some zone changes may still be in the process of being written to the journal by the time xfrin_end() destroys the relevant structure. Such a scenario leads to crashes. Fix by not destroying the zone journal structure until the entire zone transfer context is destroyed. xfrin_destroy() already conditionally calls dns_journal_destroy() and when the former is called, all asynchronous work for a given zone transfer process is guaranteed to be complete. --- lib/dns/probes.d | 2 -- lib/dns/xfrin.c | 7 ------- 2 files changed, 9 deletions(-) diff --git a/lib/dns/probes.d b/lib/dns/probes.d index 2d9f2c3d62..f5abec43ca 100644 --- a/lib/dns/probes.d +++ b/lib/dns/probes.d @@ -17,8 +17,6 @@ provider libdns { probe xfrin_connected(void *, char *, int); probe xfrin_done_callback_begin(void *, char *, int); probe xfrin_done_callback_end(void *, char *, int); - probe xfrin_journal_destroy_begin(void *, char *, int); - probe xfrin_journal_destroy_end(void *, char *, int); probe xfrin_read(void *, char *, int); probe xfrin_recv_answer(void *, char *, void *); probe xfrin_recv_done(void *, char *, int); diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 697e8507c9..9ac923f5e7 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1656,13 +1656,6 @@ get_edns_expire(dns_xfrin_t *xfr, dns_message_t *msg) { static void xfrin_end(dns_xfrin_t *xfr, isc_result_t result) { - /* Close the journal. */ - if (xfr->ixfr.journal != NULL) { - LIBDNS_XFRIN_JOURNAL_DESTROY_BEGIN(xfr, xfr->info, result); - dns_journal_destroy(&xfr->ixfr.journal); - LIBDNS_XFRIN_JOURNAL_DESTROY_END(xfr, xfr->info, result); - } - /* Inform the caller. */ if (xfr->done != NULL) { LIBDNS_XFRIN_DONE_CALLBACK_BEGIN(xfr, xfr->info, result); From 80695e9897a11e6b633931fa5c32d3807c50f805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 20 Dec 2023 17:21:14 +0100 Subject: [PATCH 2/2] Add CHANGES entry for GL #4496 --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 9a033e1573..3af1d90076 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6308. [bug] Prevent crashes caused by the zone journal getting + destroyed before all changes from an incoming IXFR are + written to it. [GL #4496] + 6307. [bug] Obtain a client->handle reference when calling async_restart. [GL #4439]