4176. [bug] Address race issues with lwresd. [RT #40284]

(cherry picked from commit 563878539af45d41143701a6d299e9b226937f5b)

diff --git a/CHANGES b/CHANGES
index f5212d81cab3d48de7babf4619a522859d4cdeb3..24728f80604410797c5fcd4cf0fdeffe2afa146f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,5 @@
+4176.  [bug]           Address race issues with lwresd. [RT #40284]
+
 4175.  [bug]           TKEY with GSS-API keys needed bigger buffers.
                        [RT #40333]
 

diff --git a/bin/named/include/named/lwdclient.h b/bin/named/include/named/lwdclient.h
index c345176a212716b7b4886b47dbdba71c0341006f..f05857fc3ff1f6e348abed67d2ba8e6130191a1a 100644 (file)
--- a/bin/named/include/named/lwdclient.h
+++ b/bin/named/include/named/lwdclient.h
@@ -186,6 +186,7 @@ struct ns_lwdclientmgr {
        lwres_context_t        *lwctx;          /*%< lightweight proto context */
        isc_task_t             *task;           /*%< owning task */
        unsigned int            flags;
+       isc_mutex_t             lock;
        ISC_LINK(ns_lwdclientmgr_t)     link;
        ISC_LIST(ns_lwdclient_t)        idle;           /*%< idle client slots */
        ISC_LIST(ns_lwdclient_t)        running;        /*%< running clients */

diff --git a/bin/named/lwdclient.c b/bin/named/lwdclient.c
index a8431340024ce538a1ddcab64ff0757693a121ee..447e5f57fbf94f2228b0b978de5a8a3178b9fc1f 100644 (file)
--- a/bin/named/lwdclient.c
+++ b/bin/named/lwdclient.c
@@ -59,12 +59,16 @@ ns_lwdclientmgr_create(ns_lwreslistener_t *listener, unsigned int nclients,
        ns_lwdclientmgr_t *cm;
        ns_lwdclient_t *client;
        unsigned int i;
-       isc_result_t result = ISC_R_FAILURE;
+       isc_result_t result;
 
        cm = isc_mem_get(lwresd->mctx, sizeof(ns_lwdclientmgr_t));
        if (cm == NULL)
                return (ISC_R_NOMEMORY);
 
+       result = isc_mutex_init(&cm->lock);
+       if (result != ISC_R_SUCCESS)
+               goto freecm;
+
        cm->listener = NULL;
        ns_lwreslistener_attach(listener, &cm->listener);
        cm->mctx = lwresd->mctx;
@@ -78,10 +82,10 @@ ns_lwdclientmgr_create(ns_lwreslistener_t *listener, unsigned int nclients,
        ISC_LIST_INIT(cm->idle);
        ISC_LIST_INIT(cm->running);
 
-       if (lwres_context_create(&cm->lwctx, cm->mctx,
-                                ns__lwresd_memalloc, ns__lwresd_memfree,
-                                LWRES_CONTEXT_SERVERMODE)
-           != ISC_R_SUCCESS)
+       result = lwres_context_create(&cm->lwctx, cm->mctx,
+                                     ns__lwresd_memalloc, ns__lwresd_memfree,
+                                     LWRES_CONTEXT_SERVERMODE);
+        if (result != ISC_R_SUCCESS)
                goto errout;
 
        for (i = 0; i < nclients; i++) {
@@ -96,8 +100,10 @@ ns_lwdclientmgr_create(ns_lwreslistener_t *listener, unsigned int nclients,
        /*
         * If we could create no clients, clean up and return.
         */
-       if (ISC_LIST_EMPTY(cm->idle))
+       if (ISC_LIST_EMPTY(cm->idle)) {
+               result = ISC_R_NOMEMORY;
                goto errout;
+       }
 
        result = isc_task_create(taskmgr, 0, &cm->task);
        if (result != ISC_R_SUCCESS)
@@ -130,6 +136,9 @@ ns_lwdclientmgr_create(ns_lwreslistener_t *listener, unsigned int nclients,
        if (cm->lwctx != NULL)
                lwres_context_destroy(&cm->lwctx);
 
+       DESTROYLOCK(&cm->lock);
+
+ freecm:
        isc_mem_put(lwresd->mctx, cm, sizeof(*cm));
        return (result);
 }
@@ -139,11 +148,14 @@ lwdclientmgr_destroy(ns_lwdclientmgr_t *cm) {
        ns_lwdclient_t *client;
        ns_lwreslistener_t *listener;
 
-       if (!SHUTTINGDOWN(cm))
+       LOCK(&cm->lock);
+       if (!SHUTTINGDOWN(cm)) {
+               UNLOCK(&cm->lock);
                return;
+       }
 
        /*
-        * run through the idle list and free the clients there.  Idle
+        * Run through the idle list and free the clients there.  Idle
         * clients do not have a recv running nor do they have any finds
         * or similar running.
         */
@@ -156,14 +168,20 @@ lwdclientmgr_destroy(ns_lwdclientmgr_t *cm) {
                client = ISC_LIST_HEAD(cm->idle);
        }
 
-       if (!ISC_LIST_EMPTY(cm->running))
+       if (!ISC_LIST_EMPTY(cm->running)) {
+               UNLOCK(&cm->lock);
                return;
+       }
+
+       UNLOCK(&cm->lock);
 
        lwres_context_destroy(&cm->lwctx);
        cm->view = NULL;
        isc_socket_detach(&cm->sock);
        isc_task_detach(&cm->task);
 
+       DESTROYLOCK(&cm->lock);
+
        listener = cm->listener;
        ns_lwreslistener_unlinkcm(listener, cm);
        ns_lwdclient_log(50, "destroying manager %p", cm);
@@ -225,8 +243,10 @@ ns_lwdclient_recv(isc_task_t *task, isc_event_t *ev) {
 
        NS_LWDCLIENT_SETRECVDONE(client);
 
+       LOCK(&cm->lock);
        INSIST((cm->flags & NS_LWDCLIENTMGR_FLAGRECVPENDING) != 0);
        cm->flags &= ~NS_LWDCLIENTMGR_FLAGRECVPENDING;
+       UNLOCK(&cm->lock);
 
        ns_lwdclient_log(50,
                         "event received: task %p, length %u, result %u (%s)",
@@ -274,26 +294,42 @@ ns_lwdclient_startrecv(ns_lwdclientmgr_t *cm) {
        ns_lwdclient_t *client;
        isc_result_t result;
        isc_region_t r;
+       isc_boolean_t destroy = ISC_FALSE;
 
+
+       LOCK(&cm->lock);
        if (SHUTTINGDOWN(cm)) {
-               lwdclientmgr_destroy(cm);
-               return (ISC_R_SUCCESS);
+               destroy = ISC_TRUE;
+               result = ISC_R_SUCCESS;
+               goto unlock;
        }
 
        /*
         * If a recv is already running, don't bother.
         */
-       if ((cm->flags & NS_LWDCLIENTMGR_FLAGRECVPENDING) != 0)
-               return (ISC_R_SUCCESS);
+       if ((cm->flags & NS_LWDCLIENTMGR_FLAGRECVPENDING) != 0) {
+               result = ISC_R_SUCCESS;
+               goto unlock;
+       }
 
        /*
         * If we have no idle slots, just return success.
         */
        client = ISC_LIST_HEAD(cm->idle);
-       if (client == NULL)
-               return (ISC_R_SUCCESS);
+       if (client == NULL) {
+               result = ISC_R_SUCCESS;
+               goto unlock;
+       }
+
        INSIST(NS_LWDCLIENT_ISIDLE(client));
 
+       /*
+        * Set the flag to say there is a recv pending.  If isc_socket_recv
+        * fails we will clear the flag otherwise it will be cleared by
+        * ns_lwdclient_recv.
+        */
+       cm->flags |= NS_LWDCLIENTMGR_FLAGRECVPENDING;
+
        /*
         * Issue the recv.  If it fails, return that it did.
         */
@@ -301,13 +337,10 @@ ns_lwdclient_startrecv(ns_lwdclientmgr_t *cm) {
        r.length = LWRES_RECVLENGTH;
        result = isc_socket_recv(cm->sock, &r, 0, cm->task, ns_lwdclient_recv,
                                 client);
-       if (result != ISC_R_SUCCESS)
-               return (result);
-
-       /*
-        * Set the flag to say we've issued a recv() call.
-        */
-       cm->flags |= NS_LWDCLIENTMGR_FLAGRECVPENDING;
+       if (result != ISC_R_SUCCESS) {
+               cm->flags &= ~NS_LWDCLIENTMGR_FLAGRECVPENDING;
+               goto unlock;
+       }
 
        /*
         * Remove the client from the idle list, and put it on the running
@@ -317,7 +350,13 @@ ns_lwdclient_startrecv(ns_lwdclientmgr_t *cm) {
        ISC_LIST_UNLINK(cm->idle, client, link);
        ISC_LIST_APPEND(cm->running, client, link);
 
-       return (ISC_R_SUCCESS);
+ unlock:
+       UNLOCK(&cm->lock);
+
+       if (destroy)
+               lwdclientmgr_destroy(cm);
+
+       return (result);
 }
 
 static void
@@ -335,6 +374,7 @@ lwdclientmgr_shutdown_callback(isc_task_t *task, isc_event_t *ev) {
         * clients do not have a recv running nor do they have any finds
         * or similar running.
         */
+       LOCK(&cm->lock);
        client = ISC_LIST_HEAD(cm->idle);
        while (client != NULL) {
                ns_lwdclient_log(50, "destroying client %p, manager %p",
@@ -343,6 +383,7 @@ lwdclientmgr_shutdown_callback(isc_task_t *task, isc_event_t *ev) {
                isc_mem_put(cm->mctx, client, sizeof(*client));
                client = ISC_LIST_HEAD(cm->idle);
        }
+       UNLOCK(&cm->lock);
 
        /*
         * Cancel any pending I/O.
@@ -353,6 +394,7 @@ lwdclientmgr_shutdown_callback(isc_task_t *task, isc_event_t *ev) {
         * Run through the running client list and kill off any finds
         * in progress.
         */
+       LOCK(&cm->lock);
        client = ISC_LIST_HEAD(cm->running);
        while (client != NULL) {
                if (client->find != client->v4find
@@ -367,6 +409,8 @@ lwdclientmgr_shutdown_callback(isc_task_t *task, isc_event_t *ev) {
 
        cm->flags |= NS_LWDCLIENTMGR_FLAGSHUTTINGDOWN;
 
+       UNLOCK(&cm->lock);
+
        isc_event_free(&ev);
 }
 
@@ -387,8 +431,10 @@ ns_lwdclient_stateidle(ns_lwdclient_t *client) {
        INSIST(client->v4find == NULL);
        INSIST(client->v6find == NULL);
 
+       LOCK(&cm->lock);
        ISC_LIST_UNLINK(cm->running, client, link);
        ISC_LIST_PREPEND(cm->idle, client, link);
+       UNLOCK(&cm->lock);
 
        NS_LWDCLIENT_SETIDLE(client);
 
@@ -464,5 +510,7 @@ ns_lwdclient_initialize(ns_lwdclient_t *client, ns_lwdclientmgr_t *cmgr) {
 
        client->pktinfo_valid = ISC_FALSE;
 
+       LOCK(&cmgr->lock);
        ISC_LIST_APPEND(cmgr->idle, client, link);
+       UNLOCK(&cmgr->lock);
 }