Fix deadlock between rndc addzone/delzone/modzone

It follows a description of the steps that were leading to the deadlock:

1. `do_addzone` calls `isc_task_beginexclusive`.

2. `isc_task_beginexclusive` waits for (N_WORKERS - 1) halted tasks,
   this blocks waiting for those (no. workers -1) workers to halt.
...
isc_task_beginexclusive(isc_task_t *task0) {
    ...
	while (manager->halted + 1 < manager->workers) {
		wake_all_queues(manager);
		WAIT(&manager->halt_cond, &manager->halt_lock);
	}
```

3. It is possible that in `task.c / dispatch()` a worker is running a
   task event, if that event blocks it will not allow this worker to
   halt.

4. `do_addzone` acquires `LOCK(&view->new_zone_lock);`,

5. `rmzone` event is called from some worker's `dispatch()`, `rmzone`
   blocks waiting for the same lock.

6. `do_addzone` calls `isc_task_beginexclusive`.

7. Deadlock triggered, since:
	- `rmzone` is wating for the lock.
	- `isc_task_beginexclusive` is waiting for (no. workers - 1) to
	   be halted
	- since `rmzone` event is blocked it won't allow the worker to halt.

To fix this, we updated do_addzone code to call isc_task_beginexclusive
before the lock is acquired, we postpone locking to the nearest required
place, same for isc_task_beginexclusive.

The same could happen with rndc modzone, so that was addressed as well.

diff --git a/bin/named/server.c b/bin/named/server.c
index 343dc6acc89c0fbcc496324d524143e881ff1554..e2b44c4b173ab4fb009e5fe6fa87ffb7b1ccea4b 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -13342,13 +13342,13 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view,
 #ifndef HAVE_LMDB
        FILE *fp = NULL;
        bool cleanup_config = false;
-#else  /* HAVE_LMDB */
+#else /* HAVE_LMDB */
        MDB_txn *txn = NULL;
        MDB_dbi dbi;
+       bool locked = false;
 
        UNUSED(zoneconf);
-       LOCK(&view->new_zone_lock);
-#endif /* HAVE_LMDB */
+#endif
 
        /* Zone shouldn't already exist */
        if (redirect) {
@@ -13368,12 +13368,16 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view,
                goto cleanup;
        }
 
+       result = isc_task_beginexclusive(server->task);
+       RUNTIME_CHECK(result == ISC_R_SUCCESS);
+
 #ifndef HAVE_LMDB
        /*
         * Make sure we can open the configuration save file
         */
        result = isc_stdio_open(view->new_zone_file, "a", &fp);
        if (result != ISC_R_SUCCESS) {
+               isc_task_endexclusive(server->task);
                TCHECK(putstr(text, "unable to create '"));
                TCHECK(putstr(text, view->new_zone_file));
                TCHECK(putstr(text, "': "));
@@ -13384,9 +13388,12 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view,
        (void)isc_stdio_close(fp);
        fp = NULL;
 #else  /* HAVE_LMDB */
+       LOCK(&view->new_zone_lock);
+       locked = true;
        /* Make sure we can open the NZD database */
        result = nzd_writable(view);
        if (result != ISC_R_SUCCESS) {
+               isc_task_endexclusive(server->task);
                TCHECK(putstr(text, "unable to open NZD database for '"));
                TCHECK(putstr(text, view->new_zone_db));
                TCHECK(putstr(text, "'"));
@@ -13395,9 +13402,6 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view,
        }
 #endif /* HAVE_LMDB */
 
-       result = isc_task_beginexclusive(server->task);
-       RUNTIME_CHECK(result == ISC_R_SUCCESS);
-
        /* Mark view unfrozen and configure zone */
        dns_view_thaw(view);
        result = configure_zone(cfg->config, zoneobj, cfg->vconfig,
@@ -13501,7 +13505,9 @@ cleanup:
        if (txn != NULL) {
                (void)nzd_close(&txn, false);
        }
-       UNLOCK(&view->new_zone_lock);
+       if (locked) {
+               UNLOCK(&view->new_zone_lock);
+       }
 #endif /* HAVE_LMDB */
 
        if (zone != NULL) {
@@ -13525,7 +13531,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);
+       bool locked = false;
 #endif /* HAVE_LMDB */
 
        /* Zone must already exist */
@@ -13571,6 +13577,8 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view,
        (void)isc_stdio_close(fp);
        fp = NULL;
 #else  /* HAVE_LMDB */
+       LOCK(&view->new_zone_lock);
+       locked = true;
        /* Make sure we can open the NZD database */
        result = nzd_writable(view);
        if (result != ISC_R_SUCCESS) {
@@ -13723,7 +13731,9 @@ cleanup:
        if (txn != NULL) {
                (void)nzd_close(&txn, false);
        }
-       UNLOCK(&view->new_zone_lock);
+       if (locked) {
+               UNLOCK(&view->new_zone_lock);
+       }
 #endif /* HAVE_LMDB */
 
        if (zone != NULL) {