| 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 <kai.kang@windriver.com> |
| |
| From 366b4e1ede8aed690e981e07137cb1cb77879c36 Mon Sep 17 00:00:00 2001 |
| From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= <michal@isc.org> |
| 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 |
| |