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 3edad716161abce73ee9c7cff8fd0e0a070f8374..ea343340ad11abe546bae7f7225f3eb9463e079d 100644 (file)
--- 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 48e3ae34b558a5d888d3f5cb9f70b6dc50e4dee5..3acb5a198355b6417682e776221674d6cd0aea73 100644 (file)
--- 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 bcf586d02bda51523441224beb388d96d566e46f..145d47883ef26bace3af03b686ddf0ba4fae8afd 100644 (file)
--- 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 9553ade3e86602d37e2e462bd1bf249b511db633..c0d69c76aa3bbee29b8d6a2873d0dbdb94a9983d 100644 (file)
--- 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 45d062ee3254b7254443c3870decaf561a23e508..4883785a22fb373d04bb90ebba7c70bcacde0e58 100644 (file)
--- 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 f080e8f52c10d361ebd89b6bc095ce0758f0b857..56d4630266fa4bb4d5b0eac3dcb919446b64c5de 100644 (file)
--- 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 27073afdb1ca340c1f272cfc4fb4ef2b14631488..5c217ad5b0eb477f5d33ce16e8cc7e17a16dc3cf 100644 (file)
--- 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 be649393ec40ef081ba0f28882668976eea14b9e..4edcfc902f00ad984ebe6a49e8454847acf8f2cd 100644 (file)
--- 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 98785997211fbede221becdaa106f9aca5038e74..58e0094f72bad8d2e3faed436f33f754954816e1 100644 (file)
--- 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 e6f7787291d7ceb05b9c938e0b9a18dc401fae1a..db582009ad02cb1f3eacf3e755f4a4fb21929907 100644 (file)
--- 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