Backport patch to fix CVE-2018-5743. Ref: https://security-tracker.debian.org/tracker/CVE-2018-5743 CVE: CVE-2018-5743 Upstream-Status: Backport [https://gitlab.isc.org/isc-projects/bind9/commit/366b4e1] Signed-off-by: Kai Kang From 366b4e1ede8aed690e981e07137cb1cb77879c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 17 Jan 2019 15:53:38 +0100 Subject: [PATCH 3/6] use reference counter for pipeline groups (v3) Track pipeline groups using a shared reference counter instead of a linked list. (cherry picked from commit 513afd33eb17d5dc41a3f0d2d38204ef8c5f6f91) (cherry picked from commit 9446629b730c59c4215f08d37fbaf810282fbccb) --- bin/named/client.c | 171 ++++++++++++++++++++----------- bin/named/include/named/client.h | 2 +- 2 files changed, 110 insertions(+), 63 deletions(-) diff --git a/bin/named/client.c b/bin/named/client.c index a7b49a0f71..277656cef0 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -299,6 +299,75 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds) { } } +/*% + * Allocate a reference counter that will track the number of client structures + * using the TCP connection that 'client' called accept() for. This counter + * will be shared between all client structures associated with this TCP + * connection. + */ +static void +pipeline_init(ns_client_t *client) { + isc_refcount_t *refs; + + REQUIRE(client->pipeline_refs == NULL); + + /* + * A global memory context is used for the allocation as different + * client structures may have different memory contexts assigned and a + * reference counter allocated here might need to be freed by a + * different client. The performance impact caused by memory context + * contention here is expected to be negligible, given that this code + * is only executed for TCP connections. + */ + refs = isc_mem_allocate(client->sctx->mctx, sizeof(*refs)); + isc_refcount_init(refs, 1); + client->pipeline_refs = refs; +} + +/*% + * Increase the count of client structures using the TCP connection that + * 'source' is associated with and put a pointer to that count in 'target', + * thus associating it with the same TCP connection. + */ +static void +pipeline_attach(ns_client_t *source, ns_client_t *target) { + int old_refs; + + REQUIRE(source->pipeline_refs != NULL); + REQUIRE(target->pipeline_refs == NULL); + + old_refs = isc_refcount_increment(source->pipeline_refs); + INSIST(old_refs > 0); + target->pipeline_refs = source->pipeline_refs; +} + +/*% + * Decrease the count of client structures using the TCP connection that + * 'client' is associated with. If this is the last client using this TCP + * connection, free the reference counter and return true; otherwise, return + * false. + */ +static bool +pipeline_detach(ns_client_t *client) { + isc_refcount_t *refs; + int old_refs; + + REQUIRE(client->pipeline_refs != NULL); + + refs = client->pipeline_refs; + client->pipeline_refs = NULL; + + old_refs = isc_refcount_decrement(refs); + INSIST(old_refs > 0); + + if (old_refs == 1) { + isc_mem_free(client->sctx->mctx, refs); + return (true); + } + + return (false); +} + /*% * Check for a deactivation or shutdown request and take appropriate * action. Returns true if either is in progress; in this case @@ -421,6 +490,40 @@ exit_check(ns_client_t *client) { client->tcpmsg_valid = false; } + if (client->tcpquota != NULL) { + if (client->pipeline_refs == NULL || + pipeline_detach(client)) + { + /* + * Only detach from the TCP client quota if + * there are no more client structures using + * this TCP connection. + * + * Note that we check 'pipeline_refs' and not + * 'pipelined' because in some cases (e.g. + * after receiving a request with an opcode + * different than QUERY) 'pipelined' is set to + * false after the reference counter gets + * allocated in pipeline_init() and we must + * still drop our reference as failing to do so + * would prevent the reference counter itself + * from being freed. + */ + isc_quota_detach(&client->tcpquota); + } else { + /* + * There are other client structures using this + * TCP connection, so we cannot detach from the + * TCP client quota to prevent excess TCP + * connections from being accepted. However, + * this client structure might later be reused + * for accepting new connections and thus must + * have its 'tcpquota' field set to NULL. + */ + client->tcpquota = NULL; + } + } + if (client->tcpsocket != NULL) { CTRACE("closetcp"); isc_socket_detach(&client->tcpsocket); @@ -434,44 +537,6 @@ exit_check(ns_client_t *client) { } } - if (client->tcpquota != NULL) { - /* - * If we are not in a pipeline group, or - * we are the last client in the group, detach from - * tcpquota; otherwise, transfer the quota to - * another client in the same group. - */ - if (!ISC_LINK_LINKED(client, glink) || - (client->glink.next == NULL && - client->glink.prev == NULL)) - { - isc_quota_detach(&client->tcpquota); - } else if (client->glink.next != NULL) { - INSIST(client->glink.next->tcpquota == NULL); - client->glink.next->tcpquota = client->tcpquota; - client->tcpquota = NULL; - } else { - INSIST(client->glink.prev->tcpquota == NULL); - client->glink.prev->tcpquota = client->tcpquota; - client->tcpquota = NULL; - } - } - - /* - * Unlink from pipeline group. - */ - if (ISC_LINK_LINKED(client, glink)) { - if (client->glink.next != NULL) { - client->glink.next->glink.prev = - client->glink.prev; - } - if (client->glink.prev != NULL) { - client->glink.prev->glink.next = - client->glink.next; - } - ISC_LINK_INIT(client, glink); - } - if (client->timerset) { (void)isc_timer_reset(client->timer, isc_timertype_inactive, @@ -3130,6 +3195,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { dns_name_init(&client->signername, NULL); client->mortal = false; client->pipelined = false; + client->pipeline_refs = NULL; client->tcpquota = NULL; client->recursionquota = NULL; client->interface = NULL; @@ -3154,7 +3220,6 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { client->formerrcache.id = 0; ISC_LINK_INIT(client, link); ISC_LINK_INIT(client, rlink); - ISC_LINK_INIT(client, glink); ISC_QLINK_INIT(client, ilink); client->keytag = NULL; client->keytag_len = 0; @@ -3341,6 +3406,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) { !allowed(&netaddr, NULL, NULL, 0, NULL, ns_g_server->keepresporder))) { + pipeline_init(client); client->pipelined = true; } @@ -3800,35 +3866,16 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock, ns_interface_attach(ifp, &client->interface); client->newstate = client->state = NS_CLIENTSTATE_WORKING; INSIST(client->recursionquota == NULL); - - /* - * Transfer TCP quota to the new client. - */ - INSIST(client->tcpquota == NULL); - INSIST(oldclient->tcpquota != NULL); - client->tcpquota = oldclient->tcpquota; - oldclient->tcpquota = NULL; - - /* - * Link to a pipeline group, creating it if needed. - */ - if (!ISC_LINK_LINKED(oldclient, glink)) { - oldclient->glink.next = NULL; - oldclient->glink.prev = NULL; - } - client->glink.next = oldclient->glink.next; - client->glink.prev = oldclient; - if (oldclient->glink.next != NULL) { - oldclient->glink.next->glink.prev = client; - } - oldclient->glink.next = client; + client->tcpquota = &client->sctx->tcpquota; client->dscp = ifp->dscp; client->attributes |= NS_CLIENTATTR_TCP; - client->pipelined = true; client->mortal = true; + pipeline_attach(oldclient, client); + client->pipelined = true; + isc_socket_attach(ifp->tcpsocket, &client->tcplistener); isc_socket_attach(sock, &client->tcpsocket); isc_socket_setname(client->tcpsocket, "worker-tcp", NULL); diff --git a/bin/named/include/named/client.h b/bin/named/include/named/client.h index 1f7973f9c5..aeed9ccdda 100644 --- a/bin/named/include/named/client.h +++ b/bin/named/include/named/client.h @@ -134,6 +134,7 @@ struct ns_client { dns_name_t *signer; /*%< NULL if not valid sig */ bool mortal; /*%< Die after handling request */ bool pipelined; /*%< TCP queries not in sequence */ + isc_refcount_t *pipeline_refs; isc_quota_t *tcpquota; isc_quota_t *recursionquota; ns_interface_t *interface; @@ -167,7 +168,6 @@ struct ns_client { ISC_LINK(ns_client_t) link; ISC_LINK(ns_client_t) rlink; - ISC_LINK(ns_client_t) glink; ISC_QLINK(ns_client_t) ilink; unsigned char cookie[8]; uint32_t expire; -- 2.20.1