diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0366061a71..5c6ed782b4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1101,8 +1101,6 @@ clang:freebsd11.4:amd64: variables: CFLAGS: "${CFLAGS_COMMON}" USER: gitlab-runner - # Temporarily disable LMDB support [GL #1976] - EXTRA_CONFIGURE: "--without-lmdb" <<: *freebsd_amd64 <<: *build_job @@ -1128,8 +1126,7 @@ unit:clang:freebsd11.4:amd64: clang:freebsd12.1:amd64: variables: CFLAGS: "${CFLAGS_COMMON}" - # Temporarily disable LMDB support [GL #1976] - EXTRA_CONFIGURE: "--enable-dnstap --without-lmdb" + EXTRA_CONFIGURE: "--enable-dnstap" USER: gitlab-runner <<: *freebsd_amd64 <<: *build_job diff --git a/CHANGES b/CHANGES index d7327b60d9..d227f93047 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +5462. [bug] Move LMDB locking from LMDB itself to named. [GL #1976] + 5461. [bug] The header STALE attribute was not being updated with the write lock being held leading to incorrect statistics. Convert the header attributes to use atomic diff --git a/bin/named/server.c b/bin/named/server.c index 4a8e3912c8..e84a8b0f8d 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7582,6 +7582,8 @@ count_newzones(dns_view_t *view, ns_cfgctx_t *nzcfg, int *num_zonesp) { "for view '%s'", view->new_zone_db, view->name); + LOCK(&view->new_zone_lock); + CHECK(nzd_count(view, &n)); *num_zonesp = n; @@ -7596,6 +7598,8 @@ cleanup: *num_zonesp = 0; } + UNLOCK(&view->new_zone_lock); + return (ISC_R_SUCCESS); } @@ -7924,6 +7928,8 @@ typedef isc_result_t (*newzone_cfg_cb_t)(const cfg_obj_t *zconfig, * Immediately interrupt processing if an error is encountered while * transforming NZD data into a zone configuration object or if "callback" * returns an error. + * + * Caller must hold 'view->new_zone_lock'. */ static isc_result_t for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, @@ -8032,8 +8038,11 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, return (ISC_R_SUCCESS); } + LOCK(&view->new_zone_lock); + result = nzd_open(view, MDB_RDONLY, &txn, &dbi); if (result != ISC_R_SUCCESS) { + UNLOCK(&view->new_zone_lock); return (ISC_R_SUCCESS); } @@ -8059,6 +8068,9 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, } (void)nzd_close(&txn, false); + + UNLOCK(&view->new_zone_lock); + return (result); } @@ -8079,6 +8091,8 @@ get_newzone_config(dns_view_t *view, const char *zonename, INSIST(zoneconfig != NULL && *zoneconfig == NULL); + LOCK(&view->new_zone_lock); + CHECK(nzd_open(view, MDB_RDONLY, &txn, &dbi)); isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, @@ -8112,6 +8126,8 @@ get_newzone_config(dns_view_t *view, const char *zonename, cleanup: (void)nzd_close(&txn, false); + UNLOCK(&view->new_zone_lock); + if (zoneconf != NULL) { cfg_obj_destroy(named_g_addparser, &zoneconf); } @@ -12569,8 +12585,6 @@ nzd_save(MDB_txn **txnp, MDB_dbi dbi, dns_zone_t *zone, nzd_setkey(&key, dns_zone_getorigin(zone), namebuf, sizeof(namebuf)); - LOCK(&view->new_zone_lock); - if (zconfig == NULL) { /* We're deleting the zone from the database */ status = mdb_del(*txnp, dbi, &key, NULL); @@ -12650,8 +12664,6 @@ cleanup: } *txnp = NULL; - UNLOCK(&view->new_zone_lock); - if (text != NULL) { isc_buffer_free(&text); } @@ -12659,6 +12671,11 @@ cleanup: return (result); } +/* + * Check whether the new zone database for 'view' can be opened for writing. + * + * Caller must hold 'view->new_zone_lock'. + */ static isc_result_t nzd_writable(dns_view_t *view) { isc_result_t result = ISC_R_SUCCESS; @@ -12688,6 +12705,11 @@ nzd_writable(dns_view_t *view) { return (result); } +/* + * Open the new zone database for 'view' and start a transaction for it. + * + * Caller must hold 'view->new_zone_lock'. + */ static isc_result_t nzd_open(dns_view_t *view, unsigned int flags, MDB_txn **txnp, MDB_dbi *dbi) { int status; @@ -12815,6 +12837,13 @@ cleanup: return (result); } +/* + * If 'commit' is true, commit the new zone database transaction pointed to by + * 'txnp'; otherwise, abort that transaction. + * + * Caller must hold 'view->new_zone_lock' for the view that the transaction + * pointed to by 'txnp' was started for. + */ static isc_result_t nzd_close(MDB_txn **txnp, bool commit) { isc_result_t result = ISC_R_SUCCESS; @@ -12837,6 +12866,12 @@ nzd_close(MDB_txn **txnp, bool commit) { return (result); } +/* + * Count the zones configured in the new zone database for 'view' and store the + * result in 'countp'. + * + * Caller must hold 'view->new_zone_lock'. + */ static isc_result_t nzd_count(dns_view_t *view, int *countp) { isc_result_t result; @@ -12884,6 +12919,8 @@ migrate_nzf(dns_view_t *view) { MDB_val key, data; ns_dzarg_t dzarg; + LOCK(&view->new_zone_lock); + /* * If NZF file doesn't exist, or NZD DB exists and already * has data, return without attempting migration. @@ -13019,6 +13056,8 @@ cleanup: result = nzd_close(&txn, commit); } + UNLOCK(&view->new_zone_lock); + if (text != NULL) { isc_buffer_free(&text); } @@ -13228,6 +13267,7 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, MDB_dbi dbi; UNUSED(zoneconf); + LOCK(&view->new_zone_lock); #endif /* HAVE_LMDB */ /* Zone shouldn't already exist */ @@ -13381,6 +13421,7 @@ cleanup: if (txn != NULL) { (void)nzd_close(&txn, false); } + UNLOCK(&view->new_zone_lock); #endif /* HAVE_LMDB */ if (zone != NULL) { @@ -13404,6 +13445,7 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, #else /* HAVE_LMDB */ MDB_txn *txn = NULL; MDB_dbi dbi; + LOCK(&view->new_zone_lock); #endif /* HAVE_LMDB */ /* Zone must already exist */ @@ -13601,6 +13643,7 @@ cleanup: if (txn != NULL) { (void)nzd_close(&txn, false); } + UNLOCK(&view->new_zone_lock); #endif /* HAVE_LMDB */ if (zone != NULL) { @@ -13764,6 +13807,7 @@ rmzone(isc_task_t *task, isc_event_t *event) { if (added && cfg != NULL) { #ifdef HAVE_LMDB /* Make sure we can open the NZD database */ + LOCK(&view->new_zone_lock); result = nzd_open(view, 0, &txn, &dbi); if (result != ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, @@ -13781,6 +13825,11 @@ rmzone(isc_task_t *task, isc_event_t *event) { "delete zone configuration: %s", isc_result_totext(result)); } + + if (txn != NULL) { + (void)nzd_close(&txn, false); + } + UNLOCK(&view->new_zone_lock); #else /* ifdef HAVE_LMDB */ result = delete_zoneconf(view, cfg->add_parser, cfg->nzf_config, dns_zone_getorigin(zone), @@ -13870,11 +13919,6 @@ rmzone(isc_task_t *task, isc_event_t *event) { } } -#ifdef HAVE_LMDB - if (txn != NULL) { - (void)nzd_close(&txn, false); - } -#endif /* ifdef HAVE_LMDB */ if (raw != NULL) { dns_zone_detach(&raw); } diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index 2c5675676b..dae8b92faa 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -251,12 +251,7 @@ struct dns_view { #ifdef HAVE_LMDB #include -/* - * MDB_NOTLS is used to prevent problems after configuration is reloaded, due - * to the way LMDB's use of thread-local storage (TLS) interacts with the BIND9 - * thread model. - */ -#define DNS_LMDB_COMMON_FLAGS (MDB_CREATE | MDB_NOSUBDIR | MDB_NOTLS) +#define DNS_LMDB_COMMON_FLAGS (MDB_CREATE | MDB_NOSUBDIR | MDB_NOLOCK) #ifndef __OpenBSD__ #define DNS_LMDB_FLAGS (DNS_LMDB_COMMON_FLAGS) #else /* __OpenBSD__ */