From: Evan Hunt Date: Fri, 7 May 2021 02:41:49 +0000 (-0700) Subject: only run tasks as privileged if taskmgr is in privileged mode X-Git-Tag: v9.17.13~8^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5c08f977913cc41bface5c52808c96ee142e6953;p=thirdparty%2Fbind9.git only run tasks as privileged if taskmgr is in privileged mode all zone loading tasks have the privileged flag, but we only want them to run as privileged tasks when the server is being initialized; if we privilege them the rest of the time, the server may hang for a long time after a reload/reconfig. so now we call isc_taskmgr_setmode() to turn privileged execution mode on or off in the task manager. isc_task_privileged() returns true if the task's privilege flag is set *and* the taskmgr is in privileged execution mode. this is used to determine in which netmgr event queue the task should be run. --- diff --git a/bin/named/server.c b/bin/named/server.c index 3edad716161..ea343340ad1 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9845,6 +9845,12 @@ view_loaded(void *arg) { "all zones loaded"); } + /* + * Clear taskmgr privileged mode now that zones are loaded. + */ + isc_taskmgr_setmode(dns_zonemgr_gettaskmgr(server->zonemgr), + isc_taskmgrmode_normal); + CHECKFATAL(dns_zonemgr_forcemaint(server->zonemgr), "forcing zone maintenance"); @@ -9867,7 +9873,7 @@ view_loaded(void *arg) { } static isc_result_t -load_zones(named_server_t *server, bool reconfig) { +load_zones(named_server_t *server, bool init, bool reconfig) { isc_result_t result; dns_view_t *view; ns_zoneload_t *zl; @@ -9924,6 +9930,19 @@ cleanup: isc_mem_put(server->mctx, zl, sizeof(*zl)); } + /* + * If we're setting up the server for the first time, + * set the task manager into privileged mode; this ensures + * that no other tasks will begin to run until after + * zone loading is complete. + * + * We do *not* want to do this in the case of reload or + * reconfig, as loading a large zone could cause the server + * to be inactive for too long a time. + */ + isc_taskmgr_setmode(named_g_taskmgr, init ? isc_taskmgrmode_privileged + : isc_taskmgrmode_normal); + isc_task_endexclusive(server->task); return (result); } @@ -9989,7 +10008,7 @@ run_server(isc_task_t *task, isc_event_t *event) { CHECKFATAL(load_configuration(named_g_conffile, server, true), "loading configuration"); - CHECKFATAL(load_zones(server, false), "loading zones"); + CHECKFATAL(load_zones(server, true, false), "loading zones"); #ifdef ENABLE_AFL named_g_run_done = true; #endif /* ifdef ENABLE_AFL */ @@ -10502,7 +10521,7 @@ reload(named_server_t *server) { CHECK(loadconfig(server)); - result = load_zones(server, false); + result = load_zones(server, false, false); if (result == ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, @@ -10871,7 +10890,7 @@ named_server_reconfigcommand(named_server_t *server) { CHECK(loadconfig(server)); - result = load_zones(server, true); + result = load_zones(server, false, true); if (result == ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 48e3ae34b55..3acb5a19835 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1735,6 +1735,12 @@ dns_zonemgr_releasezone(dns_zonemgr_t *zmgr, dns_zone_t *zone); *\li 'zone->zmgr' == NULL; */ +isc_taskmgr_t * +dns_zonemgr_gettaskmgr(dns_zonemgr_t *zmgr); +/*% + * Get the tasmkgr object attached to 'zmgr'. + */ + void dns_zonemgr_settransfersin(dns_zonemgr_t *zmgr, uint32_t value); /*%< @@ -1746,7 +1752,7 @@ dns_zonemgr_settransfersin(dns_zonemgr_t *zmgr, uint32_t value); */ uint32_t -dns_zonemgr_getttransfersin(dns_zonemgr_t *zmgr); +dns_zonemgr_gettransfersin(dns_zonemgr_t *zmgr); /*%< * Return the maximum number of simultaneous transfers in allowed. * @@ -1764,7 +1770,7 @@ dns_zonemgr_settransfersperns(dns_zonemgr_t *zmgr, uint32_t value); */ uint32_t -dns_zonemgr_getttransfersperns(dns_zonemgr_t *zmgr); +dns_zonemgr_gettransfersperns(dns_zonemgr_t *zmgr); /*%< * Return the number of transfers allowed per nameserver. * diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index bcf586d02bd..145d47883ef 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -1389,8 +1389,9 @@ dns_zonemgr_getiolimit dns_zonemgr_getnotifyrate dns_zonemgr_getserialqueryrate dns_zonemgr_getstartupnotifyrate -dns_zonemgr_getttransfersin -dns_zonemgr_getttransfersperns +dns_zonemgr_gettaskmgr +dns_zonemgr_gettransfersin +dns_zonemgr_gettransfersperns dns_zonemgr_managezone dns_zonemgr_releasezone dns_zonemgr_resumexfrs diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 9553ade3e86..c0d69c76aa3 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -18478,7 +18478,7 @@ dns_zonemgr_settransfersin(dns_zonemgr_t *zmgr, uint32_t value) { } uint32_t -dns_zonemgr_getttransfersin(dns_zonemgr_t *zmgr) { +dns_zonemgr_gettransfersin(dns_zonemgr_t *zmgr) { REQUIRE(DNS_ZONEMGR_VALID(zmgr)); return (zmgr->transfersin); @@ -18492,12 +18492,19 @@ dns_zonemgr_settransfersperns(dns_zonemgr_t *zmgr, uint32_t value) { } uint32_t -dns_zonemgr_getttransfersperns(dns_zonemgr_t *zmgr) { +dns_zonemgr_gettransfersperns(dns_zonemgr_t *zmgr) { REQUIRE(DNS_ZONEMGR_VALID(zmgr)); return (zmgr->transfersperns); } +isc_taskmgr_t * +dns_zonemgr_gettaskmgr(dns_zonemgr_t *zmgr) { + REQUIRE(DNS_ZONEMGR_VALID(zmgr)); + + return (zmgr->taskmgr); +} + /* * Try to start a new incoming zone transfer to fill a quota * slot that was just vacated. diff --git a/lib/isc/include/isc/task.h b/lib/isc/include/isc/task.h index 45d062ee325..4883785a22f 100644 --- a/lib/isc/include/isc/task.h +++ b/lib/isc/include/isc/task.h @@ -12,8 +12,8 @@ #pragma once /***** -***** Module Info -*****/ + ***** Module Info + *****/ /*! \file isc/task.h * \brief The task system provides a lightweight execution context, which is @@ -84,8 +84,8 @@ #define ISC_TASKEVENT_LASTEVENT (ISC_EVENTCLASS_TASK + 65535) /***** -***** Tasks. -*****/ + ***** Tasks. + *****/ ISC_LANG_BEGINDECLS @@ -611,15 +611,15 @@ isc_task_setprivilege(isc_task_t *task, bool priv); * but when the task manager has been set to privileged execution mode via * isc_taskmgr_setmode(), only tasks with the flag set will be executed, * and all other tasks will wait until they're done. Once all privileged - * tasks have finished executing, the task manager will automatically - * return to normal execution mode and nonprivileged task can resume. + * tasks have finished executing, the task manager resumes running + * non-privileged tasks. * * Requires: *\li 'task' is a valid task. */ bool -isc_task_privilege(isc_task_t *task); +isc_task_getprivilege(isc_task_t *task); /*%< * Returns the current value of the task's privilege flag. * @@ -627,14 +627,48 @@ isc_task_privilege(isc_task_t *task); *\li 'task' is a valid task. */ +bool +isc_task_privileged(isc_task_t *task); +/*%< + * Returns true if the task's privilege flag is set *and* the task + * manager is currently in privileged mode. + * + * Requires: + *\li 'task' is a valid task. + */ + /***** -***** Task Manager. -*****/ + ***** Task Manager. + *****/ void isc_taskmgr_attach(isc_taskmgr_t *, isc_taskmgr_t **); void isc_taskmgr_detach(isc_taskmgr_t *); +/*%< + * Attach/detach the task manager. + */ + +void +isc_taskmgr_setmode(isc_taskmgr_t *manager, isc_taskmgrmode_t mode); +isc_taskmgrmode_t +isc_taskmgr_mode(isc_taskmgr_t *manager); +/*%< + * Set/get the operating mode of the task manager. Valid modes are: + * + *\li isc_taskmgrmode_normal + *\li isc_taskmgrmode_privileged + * + * In privileged execution mode, only tasks that have had the "privilege" + * flag set via isc_task_setprivilege() can be executed. When all such + * tasks are complete, non-privileged tasks resume running. The task calling + * this function should be in task-exclusive mode; the privileged tasks + * will be run after isc_task_endexclusive() is called. + * + * Requires: + * + *\li 'manager' is a valid task manager. + */ void isc_taskmgr_setexcltask(isc_taskmgr_t *mgr, isc_task_t *task); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index f080e8f52c1..56d4630266f 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -757,7 +757,7 @@ isc_nm_task_enqueue(isc_nm_t *nm, isc_task_t *task, int threadid) { worker = &nm->workers[tid]; - if (isc_task_privilege(task)) { + if (isc_task_privileged(task)) { event = (isc__netievent_t *) isc__nm_get_netievent_privilegedtask(nm, task); } else { diff --git a/lib/isc/task.c b/lib/isc/task.c index 27073afdb1c..5c217ad5b0e 100644 --- a/lib/isc/task.c +++ b/lib/isc/task.c @@ -138,6 +138,7 @@ struct isc_taskmgr { /* Locked by task manager lock. */ unsigned int default_quantum; LIST(isc_task_t) tasks; + atomic_uint_fast32_t mode; atomic_bool exclusive_req; atomic_bool exiting; @@ -995,6 +996,7 @@ isc__taskmgr_create(isc_mem_t *mctx, unsigned int default_quantum, isc_nm_t *nm, INIT_LIST(manager->tasks); atomic_init(&manager->exiting, false); + atomic_init(&manager->mode, isc_taskmgrmode_normal); atomic_store_relaxed(&manager->exclusive_req, false); isc_mem_attach(mctx, &manager->mctx); @@ -1218,6 +1220,16 @@ isc_task_unpause(isc_task_t *task) { } } +void +isc_taskmgr_setmode(isc_taskmgr_t *manager, isc_taskmgrmode_t mode) { + atomic_store(&manager->mode, mode); +} + +isc_taskmgrmode_t +isc_taskmgr_mode(isc_taskmgr_t *manager) { + return (atomic_load(&manager->mode)); +} + void isc_task_setprivilege(isc_task_t *task, bool priv) { REQUIRE(VALID_TASK(task)); @@ -1226,12 +1238,19 @@ isc_task_setprivilege(isc_task_t *task, bool priv) { } bool -isc_task_privilege(isc_task_t *task) { +isc_task_getprivilege(isc_task_t *task) { REQUIRE(VALID_TASK(task)); return (TASK_PRIVILEGED(task)); } +bool +isc_task_privileged(isc_task_t *task) { + REQUIRE(VALID_TASK(task)); + + return (isc_taskmgr_mode(task->manager) && TASK_PRIVILEGED(task)); +} + bool isc_task_exiting(isc_task_t *task) { REQUIRE(VALID_TASK(task)); diff --git a/lib/isc/tests/task_test.c b/lib/isc/tests/task_test.c index be649393ec4..4edcfc902f0 100644 --- a/lib/isc/tests/task_test.c +++ b/lib/isc/tests/task_test.c @@ -216,18 +216,19 @@ privileged_events(void **state) { * queue without things happening while we do it. */ isc_nm_pause(netmgr); + isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_privileged); result = isc_task_create(taskmgr, 0, &task1); assert_int_equal(result, ISC_R_SUCCESS); isc_task_setname(task1, "privileged", NULL); - assert_false(isc_task_privilege(task1)); + assert_false(isc_task_getprivilege(task1)); isc_task_setprivilege(task1, true); - assert_true(isc_task_privilege(task1)); + assert_true(isc_task_getprivilege(task1)); result = isc_task_create(taskmgr, 0, &task2); assert_int_equal(result, ISC_R_SUCCESS); isc_task_setname(task2, "normal", NULL); - assert_false(isc_task_privilege(task2)); + assert_false(isc_task_getprivilege(task2)); /* First event: privileged */ event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, set, @@ -296,7 +297,7 @@ privileged_events(void **state) { assert_int_equal(atomic_load(&counter), 6); isc_task_setprivilege(task1, false); - assert_false(isc_task_privilege(task1)); + assert_false(isc_task_getprivilege(task1)); isc_task_destroy(&task1); assert_null(task1); @@ -330,18 +331,19 @@ privilege_drop(void **state) { * without things happening while we do it. */ isc_nm_pause(netmgr); + isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_privileged); result = isc_task_create(taskmgr, 0, &task1); assert_int_equal(result, ISC_R_SUCCESS); isc_task_setname(task1, "privileged", NULL); - assert_false(isc_task_privilege(task1)); + assert_false(isc_task_getprivilege(task1)); isc_task_setprivilege(task1, true); - assert_true(isc_task_privilege(task1)); + assert_true(isc_task_getprivilege(task1)); result = isc_task_create(taskmgr, 0, &task2); assert_int_equal(result, ISC_R_SUCCESS); isc_task_setname(task2, "normal", NULL); - assert_false(isc_task_privilege(task2)); + assert_false(isc_task_getprivilege(task2)); /* First event: privileged */ event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, diff --git a/lib/isc/tests/taskpool_test.c b/lib/isc/tests/taskpool_test.c index 98785997211..58e0094f72b 100644 --- a/lib/isc/tests/taskpool_test.c +++ b/lib/isc/tests/taskpool_test.c @@ -164,9 +164,9 @@ set_privilege(void **state) { assert_true(VALID_TASK(task2)); assert_true(VALID_TASK(task3)); - assert_true(isc_task_privilege(task1)); - assert_true(isc_task_privilege(task2)); - assert_true(isc_task_privilege(task3)); + assert_true(isc_task_getprivilege(task1)); + assert_true(isc_task_getprivilege(task2)); + assert_true(isc_task_getprivilege(task3)); isc_task_destroy(&task1); isc_task_destroy(&task2); diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index e6f7787291d..db582009ad0 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -635,9 +635,10 @@ isc_task_endexclusive isc_task_exiting isc_task_getcurrenttime isc_task_getcurrenttimex +isc_task_getprivilege isc_task_onshutdown isc_task_pause -isc_task_privilege +isc_task_privileged isc_task_purge isc_task_purgeevent isc_task_purgerange @@ -651,6 +652,7 @@ isc_task_shutdown isc_task_unpause isc_task_unsend isc_taskmgr_excltask +isc_taskmgr_mode @IF NOTYET isc_taskmgr_renderjson @END NOTYET @@ -658,6 +660,7 @@ isc_taskmgr_renderjson isc_taskmgr_renderxml @END LIBXML2 isc_taskmgr_setexcltask +isc_taskmgr_setmode isc_taskpool_create isc_taskpool_destroy isc_taskpool_expand