From: Mark Andrews Date: Wed, 22 Aug 2012 09:19:30 +0000 (+1000) Subject: 3370. [bug] Address use after free while shutting down. [RT #30241] X-Git-Tag: v9.10.0a1~927 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=8e0a15f42f06d1616b6f5d43658e2f90ff91cbfa;p=thirdparty%2Fbind9.git 3370. [bug] Address use after free while shutting down. [RT #30241] --- diff --git a/CHANGES b/CHANGES index 16437031f53..6545802d19a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,6 @@ -3379. [bug] nsupdate terminated unexpectedly in interactive mode +3370. [bug] Address use after free while shutting down. [RT #30241] + +3369. [bug] nsupdate terminated unexpectedly in interactive mode if built with readline support. [RT #29550] 3368. [bug] , and diff --git a/bin/named/client.c b/bin/named/client.c index 00678c63372..5144ff97607 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -501,12 +501,14 @@ exit_check(ns_client_t *client) { * the dying client inbetween. */ client->state = NS_CLIENTSTATE_INACTIVE; - if (!ns_g_clienttest) - ISC_QUEUE_PUSH(manager->inactive, client, ilink); INSIST(client->recursionquota == NULL); if (client->state == client->newstate) { client->newstate = NS_CLIENTSTATE_MAX; + if (!ns_g_clienttest && manager != NULL && + !manager->exiting) + ISC_QUEUE_PUSH(manager->inactive, client, + ilink); if (client->needshutdown) isc_task_shutdown(client->task); return (ISC_TRUE); @@ -526,6 +528,7 @@ exit_check(ns_client_t *client) { REQUIRE(client->state == NS_CLIENTSTATE_INACTIVE); INSIST(client->recursionquota == NULL); + INSIST(!ISC_QLINK_LINKED(client, ilink)); ns_query_free(client); isc_mem_put(client->mctx, client->recvbuf, RECV_BUFFER_SIZE); @@ -634,6 +637,9 @@ client_shutdown(isc_task_t *task, isc_event_t *event) { client->shutdown_arg = NULL; } + if (ISC_QLINK_LINKED(client, ilink)) + ISC_QUEUE_UNLINK(client->manager->inactive, client, ilink); + client->newstate = NS_CLIENTSTATE_FREED; client->needshutdown = ISC_FALSE; (void)exit_check(client); @@ -2124,6 +2130,8 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { #ifdef ALLOW_FILTER_AAAA client->filter_aaaa = dns_aaaa_ok; #endif + client->needshutdown = ns_g_clienttest; + ISC_EVENT_INIT(&client->ctlevent, sizeof(client->ctlevent), 0, NULL, NS_EVENT_CLIENTCONTROL, client_start, client, client, NULL, NULL); @@ -2151,8 +2159,6 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { if (result != ISC_R_SUCCESS) goto cleanup_query; - client->needshutdown = ns_g_clienttest; - CTRACE("create"); *clientp = client; @@ -2518,9 +2524,10 @@ ns_clientmgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, void ns_clientmgr_destroy(ns_clientmgr_t **managerp) { + isc_result_t result; ns_clientmgr_t *manager; ns_client_t *client; - isc_boolean_t need_destroy = ISC_FALSE; + isc_boolean_t need_destroy = ISC_FALSE, unlock = ISC_FALSE; REQUIRE(managerp != NULL); manager = *managerp; @@ -2528,11 +2535,16 @@ ns_clientmgr_destroy(ns_clientmgr_t **managerp) { MTRACE("destroy"); - LOCK(&manager->listlock); + /* + * Check for success because we may already be task-exclusive + * at this point. Only if we succeed at obtaining an exclusive + * lock now will we need to relinquish it later. + */ + result = isc_task_beginexclusive(ns_g_server->task); + if (result == ISC_R_SUCCESS) + unlock = ISC_TRUE; - LOCK(&manager->lock); manager->exiting = ISC_TRUE; - UNLOCK(&manager->lock); for (client = ISC_LIST_HEAD(manager->clients); client != NULL; @@ -2542,7 +2554,8 @@ ns_clientmgr_destroy(ns_clientmgr_t **managerp) { if (ISC_LIST_EMPTY(manager->clients)) need_destroy = ISC_TRUE; - UNLOCK(&manager->listlock); + if (unlock) + isc_task_endexclusive(ns_g_server->task); if (need_destroy) clientmgr_destroy(manager); @@ -2559,6 +2572,9 @@ get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, ns_client_t *client; MTRACE("get client"); + if (manager != NULL && manager->exiting) + return (ISC_R_SHUTTINGDOWN); + /* * Allocate a client. First try to get a recycled one; * if that fails, make a new one. diff --git a/lib/isc/include/isc/queue.h b/lib/isc/include/isc/queue.h index 1bc9d5b4d17..22e2a01e3c4 100644 --- a/lib/isc/include/isc/queue.h +++ b/lib/isc/include/isc/queue.h @@ -35,7 +35,7 @@ #define ISC_QLINK_INSIST(x) (void)0 #endif -#define ISC_QLINK(type) struct { void *prev, *next; } +#define ISC_QLINK(type) struct { type *prev, *next; } #define ISC_QLINK_INIT(elt, link) \ do { \ @@ -142,4 +142,22 @@ (ret)->link.next = (ret)->link.prev = (void *)(-1); \ } while(0) +#define ISC_QUEUE_UNLINK(queue, elt, link) \ + do { \ + ISC_QLINK_INSIST(ISC_QLINK_LINKED(elt, link)); \ + LOCK(&(queue).headlock); \ + LOCK(&(queue).taillock); \ + if ((elt)->link.prev == NULL) \ + (queue).head = (elt)->link.next; \ + else \ + (elt)->link.prev->link.next = (elt)->link.next; \ + if ((elt)->link.next == NULL) \ + (queue).tail = (elt)->link.prev; \ + else \ + (elt)->link.next->link.prev = (elt)->link.prev; \ + UNLOCK(&(queue).taillock); \ + UNLOCK(&(queue).headlock); \ + (elt)->link.next = (elt)->link.prev = (void *)(-1); \ + } while(0) + #endif /* ISC_QUEUE_H */ diff --git a/lib/isc/tests/queue_test.c b/lib/isc/tests/queue_test.c index 74620fef42a..c4a80c4c54b 100644 --- a/lib/isc/tests/queue_test.c +++ b/lib/isc/tests/queue_test.c @@ -29,10 +29,11 @@ #include "isctest.h" -typedef struct item { +typedef struct item item_t; +struct item { int value; ISC_QLINK(item_t) qlink; -} item_t; +}; typedef ISC_QUEUE(item_t) item_queue_t; @@ -107,6 +108,9 @@ ATF_TC_BODY(queue_valid, tc) { ISC_QUEUE_PUSH(queue, &five, qlink); ATF_CHECK(ISC_QLINK_LINKED(&five, qlink)); + /* Test unlink by removing one item from the middle */ + ISC_QUEUE_UNLINK(queue, &three, qlink); + ISC_QUEUE_POP(queue, qlink, p); ATF_REQUIRE(p != NULL); ATF_CHECK_EQ(p->value, 1); @@ -115,10 +119,6 @@ ATF_TC_BODY(queue_valid, tc) { ATF_REQUIRE(p != NULL); ATF_CHECK_EQ(p->value, 2); - ISC_QUEUE_POP(queue, qlink, p); - ATF_REQUIRE(p != NULL); - ATF_CHECK_EQ(p->value, 3); - ISC_QUEUE_POP(queue, qlink, p); ATF_REQUIRE(p != NULL); ATF_CHECK_EQ(p->value, 4);