use an isc_task to execute rndc commands

- using an isc_task to execute all rndc functions makes it relatively
  simple for them to acquire task exclusive mode when needed
- control_recvmessage() has been separated into two functions,
  control_recvmessage() and control_respond(). the respond function
  can be called immediately from control_recvmessage() when processing
  a nonce, or it can be called after returning from the task event
  that ran the rndc command function.

diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c
index 37a19b57b6c0aebea7e5b741ec011286cc632220..924b8a74f66b737f65f553b2b10ef972ec478bc0 100644 (file)
--- a/bin/named/controlconf.c
+++ b/bin/named/controlconf.c
@@ -27,6 +27,7 @@
 #include <isc/result.h>
 #include <isc/stdtime.h>
 #include <isc/string.h>
+#include <isc/task.h>
 #include <isc/util.h>
 
 #include <dns/result.h>
@@ -70,9 +71,17 @@ struct controlconnection {
        isccc_ccmsg_t ccmsg;
        bool ccmsg_valid;
        bool sending;
-       isc_buffer_t *buffer;
        controllistener_t *listener;
+       isccc_sexpr_t *ctrl;
+       isc_buffer_t *buffer;
+       isc_buffer_t *text;
+       isccc_sexpr_t *request;
+       isccc_sexpr_t *response;
+       uint32_t alg;
+       isccc_region_t secret;
        uint32_t nonce;
+       isc_stdtime_t now;
+       isc_result_t result;
        ISC_LINK(controlconnection_t) link;
 };
 
@@ -206,16 +215,18 @@ static void
 control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
        controlconnection_t *conn = (controlconnection_t *)arg;
        controllistener_t *listener = conn->listener;
+       isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle);
 
        REQUIRE(conn->sending);
 
        conn->sending = false;
 
+       isc_nmhandle_unref(handle);
+
        if (result == ISC_R_CANCELED) {
                return;
        } else if (result != ISC_R_SUCCESS) {
                char socktext[ISC_SOCKADDR_FORMATSIZE];
-               isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle);
 
                isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext));
                isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
@@ -247,33 +258,127 @@ log_invalid(isccc_ccmsg_t *ccmsg, isc_result_t result) {
 }
 
 static void
-control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
        controlconnection_t *conn = (controlconnection_t *)arg;
-       controllistener_t *listener = NULL;
-       controlkey_t *key = NULL;
-       isccc_sexpr_t *request = NULL;
-       isccc_sexpr_t *response = NULL;
-       uint32_t algorithm;
-       isccc_region_t secret;
-       isc_stdtime_t now;
+       controllistener_t *listener = conn->listener;
+       isccc_sexpr_t *data = NULL;
        isc_buffer_t b;
        isc_region_t r;
-       isc_buffer_t *text = NULL;
-       isc_result_t eresult;
-       isccc_sexpr_t *_ctrl = NULL;
+
+       result = isccc_cc_createresponse(conn->request, conn->now,
+                                        conn->now + 60, &conn->response);
+       if (result != ISC_R_SUCCESS) {
+               goto cleanup;
+       }
+
+       data = isccc_alist_lookup(conn->response, "_data");
+       if (data != NULL) {
+               if (isccc_cc_defineuint32(data, "result", conn->result) == NULL)
+               {
+                       goto cleanup;
+               }
+       }
+
+       if (conn->result != ISC_R_SUCCESS) {
+               if (data != NULL) {
+                       const char *estr = isc_result_totext(conn->result);
+                       if (isccc_cc_definestring(data, "err", estr) == NULL) {
+                               goto cleanup;
+                       }
+               }
+       }
+
+       if (isc_buffer_usedlength(conn->text) > 0) {
+               if (data != NULL) {
+                       char *str = (char *)isc_buffer_base(conn->text);
+                       if (isccc_cc_definestring(data, "text", str) == NULL) {
+                               goto cleanup;
+                       }
+               }
+       }
+
+       conn->ctrl = isccc_alist_lookup(conn->response, "_ctrl");
+       if (conn->ctrl == NULL ||
+           isccc_cc_defineuint32(conn->ctrl, "_nonce", conn->nonce) == NULL)
+       {
+               goto cleanup;
+       }
+
+       if (conn->buffer == NULL) {
+               isc_buffer_allocate(listener->mctx, &conn->buffer, 2 * 2048);
+       }
+
+       isc_buffer_clear(conn->buffer);
+       /* Skip the length field (4 bytes) */
+       isc_buffer_add(conn->buffer, 4);
+
+       result = isccc_cc_towire(conn->response, &conn->buffer, conn->alg,
+                                &conn->secret);
+       if (result != ISC_R_SUCCESS) {
+               goto cleanup;
+       }
+
+       isc_buffer_init(&b, conn->buffer->base, 4);
+       isc_buffer_putuint32(&b, conn->buffer->used - 4);
+
+       r.base = conn->buffer->base;
+       r.length = conn->buffer->used;
+
+       isc_nmhandle_ref(handle);
+       conn->sending = true;
+       result = isc_nm_send(handle, &r, control_senddone, conn);
+       if (result != ISC_R_SUCCESS) {
+               isc_nmhandle_unref(handle);
+               conn->sending = false;
+               goto cleanup;
+       }
+
+cleanup:
+       if (conn->response != NULL) {
+               isccc_sexpr_free(&conn->response);
+       }
+       if (conn->request != NULL) {
+               isccc_sexpr_free(&conn->request);
+       }
+
+       if (conn->secret.rstart != NULL) {
+               isc_mem_put(listener->mctx, conn->secret.rstart,
+                           REGION_SIZE(conn->secret));
+       }
+       if (conn->text != NULL) {
+               isc_buffer_free(&conn->text);
+       }
+}
+
+static void
+control_command(isc_task_t *task, isc_event_t *event) {
+       controlconnection_t *conn = event->ev_arg;
+       controllistener_t *listener = conn->listener;
+
+       UNUSED(task);
+
+       conn->result = named_control_docommand(conn->request,
+                                              listener->readonly, &conn->text);
+       control_respond(conn->handle, conn->result, conn);
+       isc_nmhandle_unref(conn->handle);
+       isc_event_free(&event);
+}
+
+static void
+control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
+       controlconnection_t *conn = (controlconnection_t *)arg;
+       controllistener_t *listener = conn->listener;
+       controlkey_t *key = NULL;
+       isc_event_t *event = NULL;
        isccc_time_t sent;
        isccc_time_t exp;
        uint32_t nonce;
-       isccc_sexpr_t *data = NULL;
 
-       listener = conn->listener;
-       algorithm = DST_ALG_UNKNOWN;
-       secret.rstart = NULL;
-       text = NULL;
+       conn->ccmsg_valid = false;
 
        /* Is the server shutting down? */
        if (listener->controls->shuttingdown) {
-               goto cleanup;
+               return;
        }
 
        if (result != ISC_R_SUCCESS) {
@@ -281,7 +386,7 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
                        log_invalid(&conn->ccmsg, result);
                }
 
-               goto cleanup;
+               return;
        }
 
        for (key = ISC_LIST_HEAD(listener->keys); key != NULL;
@@ -291,19 +396,22 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 
                ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer);
                ccregion.rend = isc_buffer_used(conn->ccmsg.buffer);
-               secret.rstart = isc_mem_get(listener->mctx, key->secret.length);
-               memmove(secret.rstart, key->secret.base, key->secret.length);
-               secret.rend = secret.rstart + key->secret.length;
-               algorithm = key->algorithm;
-               result = isccc_cc_fromwire(&ccregion, &request, algorithm,
-                                          &secret);
+               conn->secret.rstart = isc_mem_get(listener->mctx,
+                                                 key->secret.length);
+               memmove(conn->secret.rstart, key->secret.base,
+                       key->secret.length);
+               conn->secret.rend = conn->secret.rstart + key->secret.length;
+               conn->alg = key->algorithm;
+               result = isccc_cc_fromwire(&ccregion, &conn->request, conn->alg,
+                                          &conn->secret);
                if (result == ISC_R_SUCCESS) {
                        break;
                }
-               isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret));
+               isc_mem_put(listener->mctx, conn->secret.rstart,
+                           REGION_SIZE(conn->secret));
                if (result != ISCCC_R_BADAUTH) {
                        log_invalid(&conn->ccmsg, result);
-                       goto cleanup;
+                       return;
                }
        }
 
@@ -313,157 +421,106 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
        }
 
        /* We shouldn't be getting a reply. */
-       if (isccc_cc_isreply(request)) {
+       if (isccc_cc_isreply(conn->request)) {
                log_invalid(&conn->ccmsg, ISC_R_FAILURE);
-               goto cleanup_request;
+               goto cleanup;
        }
 
-       isc_stdtime_get(&now);
+       isc_stdtime_get(&conn->now);
 
        /*
         * Limit exposure to replay attacks.
         */
-       _ctrl = isccc_alist_lookup(request, "_ctrl");
-       if (!isccc_alist_alistp(_ctrl)) {
+       conn->ctrl = isccc_alist_lookup(conn->request, "_ctrl");
+       if (!isccc_alist_alistp(conn->ctrl)) {
                log_invalid(&conn->ccmsg, ISC_R_FAILURE);
-               goto cleanup_request;
+               goto cleanup;
        }
 
-       if (isccc_cc_lookupuint32(_ctrl, "_tim", &sent) == ISC_R_SUCCESS) {
-               if ((sent + CLOCKSKEW) < now || (sent - CLOCKSKEW) > now) {
+       if (isccc_cc_lookupuint32(conn->ctrl, "_tim", &sent) == ISC_R_SUCCESS) {
+               if ((sent + CLOCKSKEW) < conn->now ||
+                   (sent - CLOCKSKEW) > conn->now) {
                        log_invalid(&conn->ccmsg, ISCCC_R_CLOCKSKEW);
-                       goto cleanup_request;
+                       goto cleanup;
                }
        } else {
                log_invalid(&conn->ccmsg, ISC_R_FAILURE);
-               goto cleanup_request;
+               goto cleanup;
        }
 
        /*
         * Expire messages that are too old.
         */
-       if (isccc_cc_lookupuint32(_ctrl, "_exp", &exp) == ISC_R_SUCCESS &&
-           now > exp) {
+       if (isccc_cc_lookupuint32(conn->ctrl, "_exp", &exp) == ISC_R_SUCCESS &&
+           conn->now > exp)
+       {
                log_invalid(&conn->ccmsg, ISCCC_R_EXPIRED);
-               goto cleanup_request;
+               goto cleanup;
        }
 
        /*
         * Duplicate suppression (required for UDP).
         */
-       isccc_cc_cleansymtab(listener->controls->symtab, now);
-       result = isccc_cc_checkdup(listener->controls->symtab, request, now);
+       isccc_cc_cleansymtab(listener->controls->symtab, conn->now);
+       result = isccc_cc_checkdup(listener->controls->symtab, conn->request,
+                                  conn->now);
        if (result != ISC_R_SUCCESS) {
                if (result == ISC_R_EXISTS) {
                        result = ISCCC_R_DUPLICATE;
                }
                log_invalid(&conn->ccmsg, result);
-               goto cleanup_request;
+               goto cleanup;
        }
 
        if (conn->nonce != 0 &&
-           (isccc_cc_lookupuint32(_ctrl, "_nonce", &nonce) != ISC_R_SUCCESS ||
+           (isccc_cc_lookupuint32(conn->ctrl, "_nonce", &nonce) !=
+                    ISC_R_SUCCESS ||
             conn->nonce != nonce))
        {
                log_invalid(&conn->ccmsg, ISCCC_R_BADAUTH);
-               goto cleanup_request;
+               goto cleanup;
        }
 
-       isc_buffer_allocate(listener->mctx, &text, 2 * 2048);
+       isc_buffer_allocate(listener->mctx, &conn->text, 2 * 2048);
+
+       conn->ccmsg_valid = true;
 
-       /*
-        * Establish nonce.
-        */
        if (conn->nonce == 0) {
+               /*
+                * Establish nonce.
+                */
                while (conn->nonce == 0) {
                        isc_nonce_buf(&conn->nonce, sizeof(conn->nonce));
                }
-               eresult = ISC_R_SUCCESS;
-       } else {
-               eresult = named_control_docommand(request, listener->readonly,
-                                                 &text);
-       }
-
-       result = isccc_cc_createresponse(request, now, now + 60, &response);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_request;
-       }
-
-       data = isccc_alist_lookup(response, "_data");
-       if (data != NULL) {
-               if (isccc_cc_defineuint32(data, "result", eresult) == NULL) {
-                       goto cleanup_response;
-               }
-       }
-
-       if (eresult != ISC_R_SUCCESS) {
-               if (data != NULL) {
-                       const char *estr = isc_result_totext(eresult);
-                       if (isccc_cc_definestring(data, "err", estr) == NULL) {
-                               goto cleanup_response;
-                       }
-               }
-       }
-
-       if (isc_buffer_usedlength(text) > 0) {
-               if (data != NULL) {
-                       char *str = (char *)isc_buffer_base(text);
-                       if (isccc_cc_definestring(data, "text", str) == NULL) {
-                               goto cleanup_response;
-                       }
-               }
+               conn->result = ISC_R_SUCCESS;
+               control_respond(handle, result, conn);
+               return;
        }
 
-       _ctrl = isccc_alist_lookup(response, "_ctrl");
-       if (_ctrl == NULL ||
-           isccc_cc_defineuint32(_ctrl, "_nonce", conn->nonce) == NULL)
-       {
-               goto cleanup_response;
-       }
+       /*
+        * Trigger the command.
+        */
+       isc_nmhandle_ref(handle);
+       event = isc_event_allocate(listener->mctx, conn, NAMED_EVENT_COMMAND,
+                                  control_command, conn, sizeof(isc_event_t));
+       isc_task_send(named_g_server->task, &event);
+       return;
 
-       if (conn->buffer == NULL) {
-               isc_buffer_allocate(listener->mctx, &conn->buffer, 2 * 2048);
+cleanup:
+       if (conn->response != NULL) {
+               isccc_sexpr_free(&conn->response);
        }
-
-       isc_buffer_clear(conn->buffer);
-       /* Skip the length field (4 bytes) */
-       isc_buffer_add(conn->buffer, 4);
-
-       result = isccc_cc_towire(response, &conn->buffer, algorithm, &secret);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_response;
+       if (conn->request != NULL) {
+               isccc_sexpr_free(&conn->request);
        }
 
-       isc_buffer_init(&b, conn->buffer->base, 4);
-       isc_buffer_putuint32(&b, conn->buffer->used - 4);
-
-       r.base = conn->buffer->base;
-       r.length = conn->buffer->used;
-
-       result = isc_nm_send(handle, &r, control_senddone, conn);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup_response;
+       if (conn->secret.rstart != NULL) {
+               isc_mem_put(listener->mctx, conn->secret.rstart,
+                           REGION_SIZE(conn->secret));
        }
-       conn->sending = true;
-
-       isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret));
-       isccc_sexpr_free(&request);
-       isccc_sexpr_free(&response);
-       isc_buffer_free(&text);
-       return;
-
-cleanup_response:
-       isccc_sexpr_free(&response);
-
-cleanup_request:
-       isccc_sexpr_free(&request);
-       isc_mem_put(listener->mctx, secret.rstart, REGION_SIZE(secret));
-       if (text != NULL) {
-               isc_buffer_free(&text);
+       if (conn->text != NULL) {
+               isc_buffer_free(&conn->text);
        }
-
-cleanup:
-       conn->ccmsg_valid = false;
 }
 
 static void
@@ -524,7 +581,8 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) {
 
        *conn = (controlconnection_t){ .handle = handle,
                                       .listener = listener,
-                                      .ccmsg_valid = true };
+                                      .ccmsg_valid = true,
+                                      .alg = DST_ALG_UNKNOWN };
 
        isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg);
 

diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h
index fdc89958a33fee4dafa893b319fa263c0cc8ad95..90c9980e17bf2ce0f93080af41466aefe4697fad 100644 (file)
--- a/bin/named/include/named/server.h
+++ b/bin/named/include/named/server.h
@@ -38,6 +38,7 @@
 #define NAMED_EVENTCLASS    ISC_EVENTCLASS(0x4E43)
 #define NAMED_EVENT_RELOAD  (NAMED_EVENTCLASS + 0)
 #define NAMED_EVENT_DELZONE (NAMED_EVENTCLASS + 1)
+#define NAMED_EVENT_COMMAND (NAMED_EVENTCLASS + 2)
 
 /*%
  * Name server state.  Better here than in lots of separate global variables.
@@ -80,8 +81,6 @@ struct named_server {
        uint32_t interface_interval;
        uint32_t heartbeat_interval;
 
-       isc_mutex_t    reload_event_lock;
-       isc_event_t *  reload_event;
        named_reload_t reload_status;
 
        bool flushonshutdown;

diff --git a/bin/named/server.c b/bin/named/server.c
index 0d3111c024b25cf6cb5ca31bf7914fc859fc940a..da1566ebb86354f6dd72f8d49f1b026b6c98c1c4 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9936,14 +9936,6 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) {
                                     &server->in_roothints),
                   "setting up root hints");
 
-       isc_mutex_init(&server->reload_event_lock);
-
-       server->reload_event = isc_event_allocate(
-               named_g_mctx, server, NAMED_EVENT_RELOAD, named_server_reload,
-               server, sizeof(isc_event_t));
-       CHECKFATAL(server->reload_event == NULL ? ISC_R_NOMEMORY
-                                               : ISC_R_SUCCESS,
-                  "allocating reload event");
        server->reload_status = NAMED_RELOAD_IN_PROGRESS;
 
        /*
@@ -10113,9 +10105,6 @@ named_server_destroy(named_server_t **serverp) {
 
        dst_lib_destroy();
 
-       isc_event_free(&server->reload_event);
-       isc_mutex_destroy(&server->reload_event_lock);
-
        INSIST(ISC_LIST_EMPTY(server->kasplist));
        INSIST(ISC_LIST_EMPTY(server->viewlist));
        INSIST(ISC_LIST_EMPTY(server->cachelist));
@@ -10293,7 +10282,7 @@ cleanup:
  */
 static void
 named_server_reload(isc_task_t *task, isc_event_t *event) {
-       named_server_t *server = (named_server_t *)event->ev_arg;
+       named_server_t *server = (named_server_t *)event->ev_sender;
 
        INSIST(task == server->task);
        UNUSED(task);
@@ -10303,19 +10292,15 @@ named_server_reload(isc_task_t *task, isc_event_t *event) {
                      "received SIGHUP signal to reload zones");
        (void)reload(server);
 
-       LOCK(&server->reload_event_lock);
-       INSIST(server->reload_event == NULL);
-       server->reload_event = event;
-       UNLOCK(&server->reload_event_lock);
+       isc_event_free(&event);
 }
 
 void
 named_server_reloadwanted(named_server_t *server) {
-       LOCK(&server->reload_event_lock);
-       if (server->reload_event != NULL) {
-               isc_task_send(server->task, &server->reload_event);
-       }
-       UNLOCK(&server->reload_event_lock);
+       isc_event_t *event = isc_event_allocate(
+               named_g_mctx, server, NAMED_EVENT_RELOAD, named_server_reload,
+               NULL, sizeof(isc_event_t));
+       isc_task_send(server->task, &event);
 }
 
 void

diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c
index 7753f7f463e884d7360658de79894b7b80048bbd..ef6b31c53176c0aab13820191e3adc3a5bd3c5f5 100644 (file)
--- a/bin/rndc/rndc.c
+++ b/bin/rndc/rndc.c
@@ -373,9 +373,9 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
        if (atomic_load_acquire(&sends) == 0 &&
            atomic_load_acquire(&recvs) == 0) {
                shuttingdown = true;
+               isc_nmhandle_unref(handle);
                isc_task_shutdown(rndc_task);
                isc_app_shutdown();
-               isc_nmhandle_unref(handle);
        }
 }
 

diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c
index 5b79e6148db7a73687ccddbb9a3ff1d8ddf0590a..72e79cd9ca414c2fc456c11a243278a33c3e574a 100644 (file)
--- a/lib/isccc/ccmsg.c
+++ b/lib/isccc/ccmsg.c
@@ -49,13 +49,13 @@ recv_nonce(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
        isccc_ccmsg_t *ccmsg = arg;
        isc_result_t result;
 
+       INSIST(VALID_CCMSG(ccmsg));
+
        if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) {
                ccmsg->result = eresult;
                goto done;
        }
 
-       INSIST(VALID_CCMSG(ccmsg));
-
        if (region == NULL && eresult == ISC_R_SUCCESS) {
                ccmsg->result = ISC_R_EOF;
                goto done;
@@ -116,13 +116,13 @@ recv_message(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
        isccc_ccmsg_t *ccmsg = arg;
        size_t size;
 
+       INSIST(VALID_CCMSG(ccmsg));
+
        if (eresult == ISC_R_CANCELED || eresult == ISC_R_EOF) {
                ccmsg->result = eresult;
                goto done;
        }
 
-       INSIST(VALID_CCMSG(ccmsg));
-
        if (region == NULL && eresult == ISC_R_SUCCESS) {
                ccmsg->result = ISC_R_EOF;
                goto done;