Brad Bishop | 64c979e | 2019-11-04 13:55:29 -0500 | [diff] [blame^] | 1 | Backport patch to fix CVE-2018-5743. |
| 2 | |
| 3 | Ref: |
| 4 | https://security-tracker.debian.org/tracker/CVE-2018-5743 |
| 5 | |
| 6 | CVE: CVE-2018-5743 |
| 7 | Upstream-Status: Backport [https://gitlab.isc.org/isc-projects/bind9/commit/366b4e1] |
| 8 | |
| 9 | Signed-off-by: Kai Kang <kai.kang@windriver.com> |
| 10 | |
| 11 | From 366b4e1ede8aed690e981e07137cb1cb77879c36 Mon Sep 17 00:00:00 2001 |
| 12 | From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= <michal@isc.org> |
| 13 | Date: Thu, 17 Jan 2019 15:53:38 +0100 |
| 14 | Subject: [PATCH 3/6] use reference counter for pipeline groups (v3) |
| 15 | |
| 16 | Track pipeline groups using a shared reference counter |
| 17 | instead of a linked list. |
| 18 | |
| 19 | (cherry picked from commit 513afd33eb17d5dc41a3f0d2d38204ef8c5f6f91) |
| 20 | (cherry picked from commit 9446629b730c59c4215f08d37fbaf810282fbccb) |
| 21 | --- |
| 22 | bin/named/client.c | 171 ++++++++++++++++++++----------- |
| 23 | bin/named/include/named/client.h | 2 +- |
| 24 | 2 files changed, 110 insertions(+), 63 deletions(-) |
| 25 | |
| 26 | diff --git a/bin/named/client.c b/bin/named/client.c |
| 27 | index a7b49a0f71..277656cef0 100644 |
| 28 | --- a/bin/named/client.c |
| 29 | +++ b/bin/named/client.c |
| 30 | @@ -299,6 +299,75 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds) { |
| 31 | } |
| 32 | } |
| 33 | |
| 34 | +/*% |
| 35 | + * Allocate a reference counter that will track the number of client structures |
| 36 | + * using the TCP connection that 'client' called accept() for. This counter |
| 37 | + * will be shared between all client structures associated with this TCP |
| 38 | + * connection. |
| 39 | + */ |
| 40 | +static void |
| 41 | +pipeline_init(ns_client_t *client) { |
| 42 | + isc_refcount_t *refs; |
| 43 | + |
| 44 | + REQUIRE(client->pipeline_refs == NULL); |
| 45 | + |
| 46 | + /* |
| 47 | + * A global memory context is used for the allocation as different |
| 48 | + * client structures may have different memory contexts assigned and a |
| 49 | + * reference counter allocated here might need to be freed by a |
| 50 | + * different client. The performance impact caused by memory context |
| 51 | + * contention here is expected to be negligible, given that this code |
| 52 | + * is only executed for TCP connections. |
| 53 | + */ |
| 54 | + refs = isc_mem_allocate(client->sctx->mctx, sizeof(*refs)); |
| 55 | + isc_refcount_init(refs, 1); |
| 56 | + client->pipeline_refs = refs; |
| 57 | +} |
| 58 | + |
| 59 | +/*% |
| 60 | + * Increase the count of client structures using the TCP connection that |
| 61 | + * 'source' is associated with and put a pointer to that count in 'target', |
| 62 | + * thus associating it with the same TCP connection. |
| 63 | + */ |
| 64 | +static void |
| 65 | +pipeline_attach(ns_client_t *source, ns_client_t *target) { |
| 66 | + int old_refs; |
| 67 | + |
| 68 | + REQUIRE(source->pipeline_refs != NULL); |
| 69 | + REQUIRE(target->pipeline_refs == NULL); |
| 70 | + |
| 71 | + old_refs = isc_refcount_increment(source->pipeline_refs); |
| 72 | + INSIST(old_refs > 0); |
| 73 | + target->pipeline_refs = source->pipeline_refs; |
| 74 | +} |
| 75 | + |
| 76 | +/*% |
| 77 | + * Decrease the count of client structures using the TCP connection that |
| 78 | + * 'client' is associated with. If this is the last client using this TCP |
| 79 | + * connection, free the reference counter and return true; otherwise, return |
| 80 | + * false. |
| 81 | + */ |
| 82 | +static bool |
| 83 | +pipeline_detach(ns_client_t *client) { |
| 84 | + isc_refcount_t *refs; |
| 85 | + int old_refs; |
| 86 | + |
| 87 | + REQUIRE(client->pipeline_refs != NULL); |
| 88 | + |
| 89 | + refs = client->pipeline_refs; |
| 90 | + client->pipeline_refs = NULL; |
| 91 | + |
| 92 | + old_refs = isc_refcount_decrement(refs); |
| 93 | + INSIST(old_refs > 0); |
| 94 | + |
| 95 | + if (old_refs == 1) { |
| 96 | + isc_mem_free(client->sctx->mctx, refs); |
| 97 | + return (true); |
| 98 | + } |
| 99 | + |
| 100 | + return (false); |
| 101 | +} |
| 102 | + |
| 103 | /*% |
| 104 | * Check for a deactivation or shutdown request and take appropriate |
| 105 | * action. Returns true if either is in progress; in this case |
| 106 | @@ -421,6 +490,40 @@ exit_check(ns_client_t *client) { |
| 107 | client->tcpmsg_valid = false; |
| 108 | } |
| 109 | |
| 110 | + if (client->tcpquota != NULL) { |
| 111 | + if (client->pipeline_refs == NULL || |
| 112 | + pipeline_detach(client)) |
| 113 | + { |
| 114 | + /* |
| 115 | + * Only detach from the TCP client quota if |
| 116 | + * there are no more client structures using |
| 117 | + * this TCP connection. |
| 118 | + * |
| 119 | + * Note that we check 'pipeline_refs' and not |
| 120 | + * 'pipelined' because in some cases (e.g. |
| 121 | + * after receiving a request with an opcode |
| 122 | + * different than QUERY) 'pipelined' is set to |
| 123 | + * false after the reference counter gets |
| 124 | + * allocated in pipeline_init() and we must |
| 125 | + * still drop our reference as failing to do so |
| 126 | + * would prevent the reference counter itself |
| 127 | + * from being freed. |
| 128 | + */ |
| 129 | + isc_quota_detach(&client->tcpquota); |
| 130 | + } else { |
| 131 | + /* |
| 132 | + * There are other client structures using this |
| 133 | + * TCP connection, so we cannot detach from the |
| 134 | + * TCP client quota to prevent excess TCP |
| 135 | + * connections from being accepted. However, |
| 136 | + * this client structure might later be reused |
| 137 | + * for accepting new connections and thus must |
| 138 | + * have its 'tcpquota' field set to NULL. |
| 139 | + */ |
| 140 | + client->tcpquota = NULL; |
| 141 | + } |
| 142 | + } |
| 143 | + |
| 144 | if (client->tcpsocket != NULL) { |
| 145 | CTRACE("closetcp"); |
| 146 | isc_socket_detach(&client->tcpsocket); |
| 147 | @@ -434,44 +537,6 @@ exit_check(ns_client_t *client) { |
| 148 | } |
| 149 | } |
| 150 | |
| 151 | - if (client->tcpquota != NULL) { |
| 152 | - /* |
| 153 | - * If we are not in a pipeline group, or |
| 154 | - * we are the last client in the group, detach from |
| 155 | - * tcpquota; otherwise, transfer the quota to |
| 156 | - * another client in the same group. |
| 157 | - */ |
| 158 | - if (!ISC_LINK_LINKED(client, glink) || |
| 159 | - (client->glink.next == NULL && |
| 160 | - client->glink.prev == NULL)) |
| 161 | - { |
| 162 | - isc_quota_detach(&client->tcpquota); |
| 163 | - } else if (client->glink.next != NULL) { |
| 164 | - INSIST(client->glink.next->tcpquota == NULL); |
| 165 | - client->glink.next->tcpquota = client->tcpquota; |
| 166 | - client->tcpquota = NULL; |
| 167 | - } else { |
| 168 | - INSIST(client->glink.prev->tcpquota == NULL); |
| 169 | - client->glink.prev->tcpquota = client->tcpquota; |
| 170 | - client->tcpquota = NULL; |
| 171 | - } |
| 172 | - } |
| 173 | - |
| 174 | - /* |
| 175 | - * Unlink from pipeline group. |
| 176 | - */ |
| 177 | - if (ISC_LINK_LINKED(client, glink)) { |
| 178 | - if (client->glink.next != NULL) { |
| 179 | - client->glink.next->glink.prev = |
| 180 | - client->glink.prev; |
| 181 | - } |
| 182 | - if (client->glink.prev != NULL) { |
| 183 | - client->glink.prev->glink.next = |
| 184 | - client->glink.next; |
| 185 | - } |
| 186 | - ISC_LINK_INIT(client, glink); |
| 187 | - } |
| 188 | - |
| 189 | if (client->timerset) { |
| 190 | (void)isc_timer_reset(client->timer, |
| 191 | isc_timertype_inactive, |
| 192 | @@ -3130,6 +3195,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { |
| 193 | dns_name_init(&client->signername, NULL); |
| 194 | client->mortal = false; |
| 195 | client->pipelined = false; |
| 196 | + client->pipeline_refs = NULL; |
| 197 | client->tcpquota = NULL; |
| 198 | client->recursionquota = NULL; |
| 199 | client->interface = NULL; |
| 200 | @@ -3154,7 +3220,6 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { |
| 201 | client->formerrcache.id = 0; |
| 202 | ISC_LINK_INIT(client, link); |
| 203 | ISC_LINK_INIT(client, rlink); |
| 204 | - ISC_LINK_INIT(client, glink); |
| 205 | ISC_QLINK_INIT(client, ilink); |
| 206 | client->keytag = NULL; |
| 207 | client->keytag_len = 0; |
| 208 | @@ -3341,6 +3406,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) { |
| 209 | !allowed(&netaddr, NULL, NULL, 0, NULL, |
| 210 | ns_g_server->keepresporder))) |
| 211 | { |
| 212 | + pipeline_init(client); |
| 213 | client->pipelined = true; |
| 214 | } |
| 215 | |
| 216 | @@ -3800,35 +3866,16 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock, |
| 217 | ns_interface_attach(ifp, &client->interface); |
| 218 | client->newstate = client->state = NS_CLIENTSTATE_WORKING; |
| 219 | INSIST(client->recursionquota == NULL); |
| 220 | - |
| 221 | - /* |
| 222 | - * Transfer TCP quota to the new client. |
| 223 | - */ |
| 224 | - INSIST(client->tcpquota == NULL); |
| 225 | - INSIST(oldclient->tcpquota != NULL); |
| 226 | - client->tcpquota = oldclient->tcpquota; |
| 227 | - oldclient->tcpquota = NULL; |
| 228 | - |
| 229 | - /* |
| 230 | - * Link to a pipeline group, creating it if needed. |
| 231 | - */ |
| 232 | - if (!ISC_LINK_LINKED(oldclient, glink)) { |
| 233 | - oldclient->glink.next = NULL; |
| 234 | - oldclient->glink.prev = NULL; |
| 235 | - } |
| 236 | - client->glink.next = oldclient->glink.next; |
| 237 | - client->glink.prev = oldclient; |
| 238 | - if (oldclient->glink.next != NULL) { |
| 239 | - oldclient->glink.next->glink.prev = client; |
| 240 | - } |
| 241 | - oldclient->glink.next = client; |
| 242 | + client->tcpquota = &client->sctx->tcpquota; |
| 243 | |
| 244 | client->dscp = ifp->dscp; |
| 245 | |
| 246 | client->attributes |= NS_CLIENTATTR_TCP; |
| 247 | - client->pipelined = true; |
| 248 | client->mortal = true; |
| 249 | |
| 250 | + pipeline_attach(oldclient, client); |
| 251 | + client->pipelined = true; |
| 252 | + |
| 253 | isc_socket_attach(ifp->tcpsocket, &client->tcplistener); |
| 254 | isc_socket_attach(sock, &client->tcpsocket); |
| 255 | isc_socket_setname(client->tcpsocket, "worker-tcp", NULL); |
| 256 | diff --git a/bin/named/include/named/client.h b/bin/named/include/named/client.h |
| 257 | index 1f7973f9c5..aeed9ccdda 100644 |
| 258 | --- a/bin/named/include/named/client.h |
| 259 | +++ b/bin/named/include/named/client.h |
| 260 | @@ -134,6 +134,7 @@ struct ns_client { |
| 261 | dns_name_t *signer; /*%< NULL if not valid sig */ |
| 262 | bool mortal; /*%< Die after handling request */ |
| 263 | bool pipelined; /*%< TCP queries not in sequence */ |
| 264 | + isc_refcount_t *pipeline_refs; |
| 265 | isc_quota_t *tcpquota; |
| 266 | isc_quota_t *recursionquota; |
| 267 | ns_interface_t *interface; |
| 268 | @@ -167,7 +168,6 @@ struct ns_client { |
| 269 | |
| 270 | ISC_LINK(ns_client_t) link; |
| 271 | ISC_LINK(ns_client_t) rlink; |
| 272 | - ISC_LINK(ns_client_t) glink; |
| 273 | ISC_QLINK(ns_client_t) ilink; |
| 274 | unsigned char cookie[8]; |
| 275 | uint32_t expire; |
| 276 | -- |
| 277 | 2.20.1 |
| 278 | |