From: Ondřej Surý Date: Fri, 8 May 2026 05:46:03 +0000 (+0200) Subject: Fix data race in async master dump/load context publication X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fbf5ecb5a8580d12bdf7d21813aa80d2569fe39c;p=thirdparty%2Fbind9.git Fix data race in async master dump/load context publication Bouncing the offload itself to the target loop let the after-work callback fire on the target thread and run the user's done callback before the calling thread had published *dctxp / *lctxp. Enqueue on the calling loop and bounce only the done callback instead, so the publish is sequenced before the cross-thread hand-off by construction and cannot be reintroduced by reordering the entry-point body. (cherry picked from commit 8ae464d552a4f9b6dddeb068cecb101e071bdd6e) --- diff --git a/lib/dns/master.c b/lib/dns/master.c index 2e89d28d2ba..d7514121a13 100644 --- a/lib/dns/master.c +++ b/lib/dns/master.c @@ -108,6 +108,8 @@ struct dns_loadctx { dns_loaddonefunc_t done; void *done_arg; + isc_loop_t *loop; + /* Common methods */ isc_result_t (*openfile)(dns_loadctx_t *lctx, const char *filename); isc_result_t (*load)(dns_loadctx_t *lctx); @@ -2663,33 +2665,38 @@ cleanup: return result; } +/* + * The combination of isc_work_enqueue() on the current loop and callback on + * lctx->loop ensures the correct ordering: + * + * 1. dns_master_loadfileasync() calls isc_work_enqueue() on the current loop. + * 2. master_load() runs asynchronously and can finish before the entry point + * returns; master_load_done() is queued on the current loop and cannot run + * until the entry point returns. + * 3. The entry point publishes *lctxp. + * 4. master_load_done() runs on the current loop and hands off to lctx->loop. + * 5. lctx->done() runs on lctx->loop asynchronously. + */ static void -load(void *arg) { +master_load(void *arg) { dns_loadctx_t *lctx = arg; lctx->result = (lctx->load)(lctx); } static void -load_done(void *arg) { +master_load_callback(void *arg) { dns_loadctx_t *lctx = arg; (lctx->done)(lctx->done_arg, lctx->result); + isc_loop_detach(&lctx->loop); dns_loadctx_detach(&lctx); } static void -load_enqueue(void *lctx) { - isc_work_enqueue(isc_loop(), load, load_done, lctx); -} +master_load_done(void *arg) { + dns_loadctx_t *lctx = arg; -static void -dns_loadctx_enqueue(isc_loop_t *loop, dns_loadctx_t *lctx) { - dns_loadctx_ref(lctx); - if (loop == isc_loop()) { - load_enqueue(lctx); - } else { - isc_async_run(loop, load_enqueue, lctx); - } + isc_async_run(lctx->loop, master_load_callback, lctx); } isc_result_t @@ -2720,7 +2727,10 @@ dns_master_loadfileasync(const char *master_file, dns_name_t *top, return result; } - dns_loadctx_enqueue(loop, lctx); + dns_loadctx_ref(lctx); + isc_loop_attach(loop, &lctx->loop); + isc_work_enqueue(isc_loop(), master_load, master_load_done, lctx); + *lctxp = lctx; return ISC_R_SUCCESS; diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index d1cb0371176..5721e63e5b5 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -235,6 +235,7 @@ static char tabs[N_TABS + 1] = "\t\t\t\t\t\t\t\t\t\t"; struct dns_dumpctx { unsigned int magic; isc_mem_t *mctx; + isc_loop_t *loop; isc_mutex_t lock; isc_refcount_t references; atomic_bool canceled; @@ -1487,10 +1488,20 @@ closeandrename(FILE *f, isc_result_t result, const char *temp, } /* - * This will run in a libuv threadpool thread. + * The combination of isc_work_enqueue() on the current loop and callback on + * dctx->loop ensures the correct ordering: + * + * 1. dns_master_dumptostreamasync() (or dns_master_dumpasync()) calls + * isc_work_enqueue() on the current loop. + * 2. master_dump() runs asynchronously and can finish before the entry point + * returns; master_dump_done() is queued on the current loop and cannot run + * until the entry point returns. + * 3. The entry point publishes *dctxp. + * 4. master_dump_done() runs on the current loop and hands off to dctx->loop. + * 5. dctx->done() runs on dctx->loop asynchronously. */ static void -master_dump_cb(void *data) { +master_dump(void *data) { isc_result_t result = ISC_R_UNSET; dns_dumpctx_t *dctx = data; REQUIRE(DNS_DCTX_VALID(dctx)); @@ -1515,17 +1526,22 @@ master_dump_cb(void *data) { dctx->result = result; } -/* - * This will run in a loop manager thread when the dump is complete. - */ static void -master_dump_done_cb(void *data) { +master_dump_callback(void *data) { dns_dumpctx_t *dctx = data; (dctx->done)(dctx->done_arg, dctx->result); + isc_loop_detach(&dctx->loop); dns_dumpctx_detach(&dctx); } +static void +master_dump_done(void *data) { + dns_dumpctx_t *dctx = data; + + isc_async_run(dctx->loop, master_dump_callback, dctx); +} + static isc_result_t dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, FILE *f, dns_dumpctx_t **dctxp, @@ -1756,21 +1772,6 @@ cleanup: return result; } -static void -master_dump_enqueue(void *dctx) { - isc_work_enqueue(isc_loop(), master_dump_cb, master_dump_done_cb, dctx); -} - -static void -dns_dumpctx_enqueue(isc_loop_t *loop, dns_dumpctx_t *dctx) { - dns_dumpctx_ref(dctx); - if (loop == isc_loop()) { - master_dump_enqueue(dctx); - } else { - isc_async_run(loop, master_dump_enqueue, dctx); - } -} - isc_result_t dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, @@ -1792,7 +1793,10 @@ dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, dctx->done = done; dctx->done_arg = done_arg; - dns_dumpctx_enqueue(loop, dctx); + dns_dumpctx_ref(dctx); + isc_loop_attach(loop, &dctx->loop); + isc_work_enqueue(isc_loop(), master_dump, master_dump_done, dctx); + *dctxp = dctx; return ISC_R_SUCCESS; @@ -1887,7 +1891,10 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, dctx->file = file; dctx->tmpfile = tempname; - dns_dumpctx_enqueue(loop, dctx); + dns_dumpctx_ref(dctx); + isc_loop_attach(loop, &dctx->loop); + isc_work_enqueue(isc_loop(), master_dump, master_dump_done, dctx); + *dctxp = dctx; return ISC_R_SUCCESS;