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/c47ccf6] |
| 8 | |
| 9 | Signed-off-by: Kai Kang <kai.kang@windriver.com> |
| 10 | |
| 11 | From c47ccf630f147378568b33e8fdb7b754f228c346 Mon Sep 17 00:00:00 2001 |
| 12 | From: Evan Hunt <each@isc.org> |
| 13 | Date: Fri, 5 Apr 2019 16:26:05 -0700 |
| 14 | Subject: [PATCH 5/6] refactor tcpquota and pipeline refs; allow special-case |
| 15 | overrun in isc_quota |
| 16 | |
| 17 | - if the TCP quota has been exceeded but there are no clients listening |
| 18 | for new connections on the interface, we can now force attachment to the |
| 19 | quota using isc_quota_force(), instead of carrying on with the quota not |
| 20 | attached. |
| 21 | - the TCP client quota is now referenced via a reference-counted |
| 22 | 'ns_tcpconn' object, one of which is created whenever a client begins |
| 23 | listening for new connections, and attached to by members of that |
| 24 | client's pipeline group. when the last reference to the tcpconn |
| 25 | object is detached, it is freed and the TCP quota slot is released. |
| 26 | - reduce code duplication by adding mark_tcp_active() function. |
| 27 | - convert counters to atomic. |
| 28 | |
| 29 | (cherry picked from commit 7e8222378ca24f1302a0c1c638565050ab04681b) |
| 30 | (cherry picked from commit 4939451275722bfda490ea86ca13e84f6bc71e46) |
| 31 | (cherry picked from commit 13f7c918b8720d890408f678bd73c20e634539d9) |
| 32 | --- |
| 33 | bin/named/client.c | 444 +++++++++++-------------- |
| 34 | bin/named/include/named/client.h | 12 +- |
| 35 | bin/named/include/named/interfacemgr.h | 6 +- |
| 36 | bin/named/interfacemgr.c | 1 + |
| 37 | lib/isc/include/isc/quota.h | 7 + |
| 38 | lib/isc/quota.c | 33 +- |
| 39 | lib/isc/win32/libisc.def.in | 1 + |
| 40 | 7 files changed, 236 insertions(+), 268 deletions(-) |
| 41 | |
| 42 | diff --git a/bin/named/client.c b/bin/named/client.c |
| 43 | index 61e96dd28c..d826ab32bf 100644 |
| 44 | --- a/bin/named/client.c |
| 45 | +++ b/bin/named/client.c |
| 46 | @@ -244,8 +244,7 @@ static void client_start(isc_task_t *task, isc_event_t *event); |
| 47 | static void client_request(isc_task_t *task, isc_event_t *event); |
| 48 | static void ns_client_dumpmessage(ns_client_t *client, const char *reason); |
| 49 | static isc_result_t get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, |
| 50 | - dns_dispatch_t *disp, ns_client_t *oldclient, |
| 51 | - bool tcp); |
| 52 | + dns_dispatch_t *disp, bool tcp); |
| 53 | static isc_result_t get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, |
| 54 | isc_socket_t *sock, ns_client_t *oldclient); |
| 55 | static inline bool |
| 56 | @@ -301,16 +300,32 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds) { |
| 57 | } |
| 58 | |
| 59 | /*% |
| 60 | - * Allocate a reference counter that will track the number of client structures |
| 61 | - * using the TCP connection that 'client' called accept() for. This counter |
| 62 | - * will be shared between all client structures associated with this TCP |
| 63 | - * connection. |
| 64 | + * Allocate a reference-counted object that will maintain a single pointer to |
| 65 | + * the (also reference-counted) TCP client quota, shared between all the |
| 66 | + * clients processing queries on a single TCP connection, so that all |
| 67 | + * clients sharing the one socket will together consume only one slot in |
| 68 | + * the 'tcp-clients' quota. |
| 69 | */ |
| 70 | -static void |
| 71 | -pipeline_init(ns_client_t *client) { |
| 72 | - isc_refcount_t *refs; |
| 73 | +static isc_result_t |
| 74 | +tcpconn_init(ns_client_t *client, bool force) { |
| 75 | + isc_result_t result; |
| 76 | + isc_quota_t *quota = NULL; |
| 77 | + ns_tcpconn_t *tconn = NULL; |
| 78 | |
| 79 | - REQUIRE(client->pipeline_refs == NULL); |
| 80 | + REQUIRE(client->tcpconn == NULL); |
| 81 | + |
| 82 | + /* |
| 83 | + * Try to attach to the quota first, so we won't pointlessly |
| 84 | + * allocate memory for a tcpconn object if we can't get one. |
| 85 | + */ |
| 86 | + if (force) { |
| 87 | + result = isc_quota_force(&ns_g_server->tcpquota, "a); |
| 88 | + } else { |
| 89 | + result = isc_quota_attach(&ns_g_server->tcpquota, "a); |
| 90 | + } |
| 91 | + if (result != ISC_R_SUCCESS) { |
| 92 | + return (result); |
| 93 | + } |
| 94 | |
| 95 | /* |
| 96 | * A global memory context is used for the allocation as different |
| 97 | @@ -320,78 +335,80 @@ pipeline_init(ns_client_t *client) { |
| 98 | * contention here is expected to be negligible, given that this code |
| 99 | * is only executed for TCP connections. |
| 100 | */ |
| 101 | - refs = isc_mem_allocate(ns_g_mctx, sizeof(*refs)); |
| 102 | - isc_refcount_init(refs, 1); |
| 103 | - client->pipeline_refs = refs; |
| 104 | + tconn = isc_mem_allocate(ns_g_mctx, sizeof(*tconn)); |
| 105 | + |
| 106 | + isc_refcount_init(&tconn->refs, 1); |
| 107 | + tconn->tcpquota = quota; |
| 108 | + quota = NULL; |
| 109 | + tconn->pipelined = false; |
| 110 | + |
| 111 | + client->tcpconn = tconn; |
| 112 | + |
| 113 | + return (ISC_R_SUCCESS); |
| 114 | } |
| 115 | |
| 116 | /*% |
| 117 | - * Increase the count of client structures using the TCP connection that |
| 118 | - * 'source' is associated with and put a pointer to that count in 'target', |
| 119 | - * thus associating it with the same TCP connection. |
| 120 | + * Increase the count of client structures sharing the TCP connection |
| 121 | + * that 'source' is associated with; add a pointer to the same tcpconn |
| 122 | + * to 'target', thus associating it with the same TCP connection. |
| 123 | */ |
| 124 | static void |
| 125 | -pipeline_attach(ns_client_t *source, ns_client_t *target) { |
| 126 | +tcpconn_attach(ns_client_t *source, ns_client_t *target) { |
| 127 | int refs; |
| 128 | |
| 129 | - REQUIRE(source->pipeline_refs != NULL); |
| 130 | - REQUIRE(target->pipeline_refs == NULL); |
| 131 | + REQUIRE(source->tcpconn != NULL); |
| 132 | + REQUIRE(target->tcpconn == NULL); |
| 133 | + REQUIRE(source->tcpconn->pipelined); |
| 134 | |
| 135 | - isc_refcount_increment(source->pipeline_refs, &refs); |
| 136 | + isc_refcount_increment(&source->tcpconn->refs, &refs); |
| 137 | INSIST(refs > 1); |
| 138 | - target->pipeline_refs = source->pipeline_refs; |
| 139 | + target->tcpconn = source->tcpconn; |
| 140 | } |
| 141 | |
| 142 | /*% |
| 143 | - * Decrease the count of client structures using the TCP connection that |
| 144 | + * Decrease the count of client structures sharing the TCP connection that |
| 145 | * 'client' is associated with. If this is the last client using this TCP |
| 146 | - * connection, free the reference counter and return true; otherwise, return |
| 147 | - * false. |
| 148 | + * connection, we detach from the TCP quota and free the tcpconn |
| 149 | + * object. Either way, client->tcpconn is set to NULL. |
| 150 | */ |
| 151 | -static bool |
| 152 | -pipeline_detach(ns_client_t *client) { |
| 153 | - isc_refcount_t *refcount; |
| 154 | +static void |
| 155 | +tcpconn_detach(ns_client_t *client) { |
| 156 | + ns_tcpconn_t *tconn = NULL; |
| 157 | int refs; |
| 158 | |
| 159 | - REQUIRE(client->pipeline_refs != NULL); |
| 160 | - |
| 161 | - refcount = client->pipeline_refs; |
| 162 | - client->pipeline_refs = NULL; |
| 163 | + REQUIRE(client->tcpconn != NULL); |
| 164 | |
| 165 | - isc_refcount_decrement(refcount, refs); |
| 166 | + tconn = client->tcpconn; |
| 167 | + client->tcpconn = NULL; |
| 168 | |
| 169 | + isc_refcount_decrement(&tconn->refs, &refs); |
| 170 | if (refs == 0) { |
| 171 | - isc_mem_free(ns_g_mctx, refs); |
| 172 | - return (true); |
| 173 | + isc_quota_detach(&tconn->tcpquota); |
| 174 | + isc_mem_free(ns_g_mctx, tconn); |
| 175 | } |
| 176 | - |
| 177 | - return (false); |
| 178 | } |
| 179 | |
| 180 | -/* |
| 181 | - * Detach a client from the TCP client quota if appropriate, and set |
| 182 | - * the quota pointer to NULL. |
| 183 | - * |
| 184 | - * Sometimes when the TCP client quota is exhausted but there are no other |
| 185 | - * clients servicing the interface, a client will be allowed to continue |
| 186 | - * running despite not having been attached to the quota. In this event, |
| 187 | - * the TCP quota was never attached to the client, so when the client (or |
| 188 | - * associated pipeline group) shuts down, the quota must NOT be detached. |
| 189 | +/*% |
| 190 | + * Mark a client as active and increment the interface's 'ntcpactive' |
| 191 | + * counter, as a signal that there is at least one client servicing |
| 192 | + * TCP queries for the interface. If we reach the TCP client quota at |
| 193 | + * some point, this will be used to determine whether a quota overrun |
| 194 | + * should be permitted. |
| 195 | * |
| 196 | - * Otherwise, if the quota pointer is set, it should be detached. If not |
| 197 | - * set at all, we just return without doing anything. |
| 198 | + * Marking the client active with the 'tcpactive' flag ensures proper |
| 199 | + * accounting, by preventing us from incrementing or decrementing |
| 200 | + * 'ntcpactive' more than once per client. |
| 201 | */ |
| 202 | static void |
| 203 | -tcpquota_disconnect(ns_client_t *client) { |
| 204 | - if (client->tcpquota == NULL) { |
| 205 | - return; |
| 206 | - } |
| 207 | - |
| 208 | - if (client->tcpattached) { |
| 209 | - isc_quota_detach(&client->tcpquota); |
| 210 | - client->tcpattached = false; |
| 211 | - } else { |
| 212 | - client->tcpquota = NULL; |
| 213 | +mark_tcp_active(ns_client_t *client, bool active) { |
| 214 | + if (active && !client->tcpactive) { |
| 215 | + isc_atomic_xadd(&client->interface->ntcpactive, 1); |
| 216 | + client->tcpactive = active; |
| 217 | + } else if (!active && client->tcpactive) { |
| 218 | + uint32_t old = |
| 219 | + isc_atomic_xadd(&client->interface->ntcpactive, -1); |
| 220 | + INSIST(old > 0); |
| 221 | + client->tcpactive = active; |
| 222 | } |
| 223 | } |
| 224 | |
| 225 | @@ -484,7 +501,8 @@ exit_check(ns_client_t *client) { |
| 226 | INSIST(client->recursionquota == NULL); |
| 227 | |
| 228 | if (NS_CLIENTSTATE_READING == client->newstate) { |
| 229 | - if (!client->pipelined) { |
| 230 | + INSIST(client->tcpconn != NULL); |
| 231 | + if (!client->tcpconn->pipelined) { |
| 232 | client_read(client); |
| 233 | client->newstate = NS_CLIENTSTATE_MAX; |
| 234 | return (true); /* We're done. */ |
| 235 | @@ -507,8 +525,8 @@ exit_check(ns_client_t *client) { |
| 236 | dns_tcpmsg_cancelread(&client->tcpmsg); |
| 237 | } |
| 238 | |
| 239 | - if (client->nreads != 0) { |
| 240 | - /* Still waiting for read cancel completion. */ |
| 241 | + /* Still waiting for read cancel completion. */ |
| 242 | + if (client->nreads > 0) { |
| 243 | return (true); |
| 244 | } |
| 245 | |
| 246 | @@ -518,43 +536,45 @@ exit_check(ns_client_t *client) { |
| 247 | } |
| 248 | |
| 249 | /* |
| 250 | - * Detach from pipeline group and from TCP client quota, |
| 251 | - * if appropriate. |
| 252 | + * Soon the client will be ready to accept a new TCP |
| 253 | + * connection or UDP request, but we may have enough |
| 254 | + * clients doing that already. Check whether this client |
| 255 | + * needs to remain active and allow it go inactive if |
| 256 | + * not. |
| 257 | * |
| 258 | - * - If no pipeline group is active, attempt to |
| 259 | - * detach from the TCP client quota. |
| 260 | + * UDP clients always go inactive at this point, but a TCP |
| 261 | + * client may need to stay active and return to READY |
| 262 | + * state if no other clients are available to listen |
| 263 | + * for TCP requests on this interface. |
| 264 | * |
| 265 | - * - If a pipeline group is active, detach from it; |
| 266 | - * if the return code indicates that there no more |
| 267 | - * clients left if this pipeline group, we also detach |
| 268 | - * from the TCP client quota. |
| 269 | - * |
| 270 | - * - Otherwise we don't try to detach, we just set the |
| 271 | - * TCP quota pointer to NULL if it wasn't NULL already. |
| 272 | - * |
| 273 | - * tcpquota_disconnect() will set tcpquota to NULL, either |
| 274 | - * by detaching it or by assignment, depending on the |
| 275 | - * needs of the client. See the comments on that function |
| 276 | - * for further information. |
| 277 | + * Regardless, if we're going to FREED state, that means |
| 278 | + * the system is shutting down and we don't need to |
| 279 | + * retain clients. |
| 280 | */ |
| 281 | - if (client->pipeline_refs == NULL || pipeline_detach(client)) { |
| 282 | - tcpquota_disconnect(client); |
| 283 | - } else { |
| 284 | - client->tcpquota = NULL; |
| 285 | - client->tcpattached = false; |
| 286 | + if (client->mortal && TCP_CLIENT(client) && |
| 287 | + client->newstate != NS_CLIENTSTATE_FREED && |
| 288 | + !ns_g_clienttest && |
| 289 | + isc_atomic_xadd(&client->interface->ntcpaccepting, 0) == 0) |
| 290 | + { |
| 291 | + /* Nobody else is accepting */ |
| 292 | + client->mortal = false; |
| 293 | + client->newstate = NS_CLIENTSTATE_READY; |
| 294 | + } |
| 295 | + |
| 296 | + /* |
| 297 | + * Detach from TCP connection and TCP client quota, |
| 298 | + * if appropriate. If this is the last reference to |
| 299 | + * the TCP connection in our pipeline group, the |
| 300 | + * TCP quota slot will be released. |
| 301 | + */ |
| 302 | + if (client->tcpconn) { |
| 303 | + tcpconn_detach(client); |
| 304 | } |
| 305 | |
| 306 | if (client->tcpsocket != NULL) { |
| 307 | CTRACE("closetcp"); |
| 308 | isc_socket_detach(&client->tcpsocket); |
| 309 | - |
| 310 | - if (client->tcpactive) { |
| 311 | - LOCK(&client->interface->lock); |
| 312 | - INSIST(client->interface->ntcpactive > 0); |
| 313 | - client->interface->ntcpactive--; |
| 314 | - UNLOCK(&client->interface->lock); |
| 315 | - client->tcpactive = false; |
| 316 | - } |
| 317 | + mark_tcp_active(client, false); |
| 318 | } |
| 319 | |
| 320 | if (client->timerset) { |
| 321 | @@ -567,35 +587,6 @@ exit_check(ns_client_t *client) { |
| 322 | client->peeraddr_valid = false; |
| 323 | |
| 324 | client->state = NS_CLIENTSTATE_READY; |
| 325 | - INSIST(client->recursionquota == NULL); |
| 326 | - |
| 327 | - /* |
| 328 | - * Now the client is ready to accept a new TCP connection |
| 329 | - * or UDP request, but we may have enough clients doing |
| 330 | - * that already. Check whether this client needs to remain |
| 331 | - * active and force it to go inactive if not. |
| 332 | - * |
| 333 | - * UDP clients go inactive at this point, but a TCP client |
| 334 | - * may need to remain active and go into ready state if |
| 335 | - * no other clients are available to listen for TCP |
| 336 | - * requests on this interface or (in the case of pipelined |
| 337 | - * clients) to read for additional messages on the current |
| 338 | - * connection. |
| 339 | - */ |
| 340 | - if (client->mortal && TCP_CLIENT(client) && !ns_g_clienttest) { |
| 341 | - LOCK(&client->interface->lock); |
| 342 | - if ((client->interface->ntcpaccepting == 0 || |
| 343 | - (client->pipelined && |
| 344 | - client->interface->ntcpactive < 2)) && |
| 345 | - client->newstate != NS_CLIENTSTATE_FREED) |
| 346 | - { |
| 347 | - client->mortal = false; |
| 348 | - client->newstate = NS_CLIENTSTATE_READY; |
| 349 | - } |
| 350 | - UNLOCK(&client->interface->lock); |
| 351 | - } |
| 352 | - |
| 353 | - client->pipelined = false; |
| 354 | |
| 355 | /* |
| 356 | * We don't need the client; send it to the inactive |
| 357 | @@ -630,7 +621,7 @@ exit_check(ns_client_t *client) { |
| 358 | } |
| 359 | |
| 360 | /* Still waiting for accept cancel completion. */ |
| 361 | - if (! (client->naccepts == 0)) { |
| 362 | + if (client->naccepts > 0) { |
| 363 | return (true); |
| 364 | } |
| 365 | |
| 366 | @@ -641,7 +632,7 @@ exit_check(ns_client_t *client) { |
| 367 | } |
| 368 | |
| 369 | /* Still waiting for recv cancel completion. */ |
| 370 | - if (! (client->nrecvs == 0)) { |
| 371 | + if (client->nrecvs > 0) { |
| 372 | return (true); |
| 373 | } |
| 374 | |
| 375 | @@ -654,14 +645,7 @@ exit_check(ns_client_t *client) { |
| 376 | INSIST(client->recursionquota == NULL); |
| 377 | if (client->tcplistener != NULL) { |
| 378 | isc_socket_detach(&client->tcplistener); |
| 379 | - |
| 380 | - if (client->tcpactive) { |
| 381 | - LOCK(&client->interface->lock); |
| 382 | - INSIST(client->interface->ntcpactive > 0); |
| 383 | - client->interface->ntcpactive--; |
| 384 | - UNLOCK(&client->interface->lock); |
| 385 | - client->tcpactive = false; |
| 386 | - } |
| 387 | + mark_tcp_active(client, false); |
| 388 | } |
| 389 | if (client->udpsocket != NULL) { |
| 390 | isc_socket_detach(&client->udpsocket); |
| 391 | @@ -816,7 +800,7 @@ client_start(isc_task_t *task, isc_event_t *event) { |
| 392 | return; |
| 393 | |
| 394 | if (TCP_CLIENT(client)) { |
| 395 | - if (client->pipelined) { |
| 396 | + if (client->tcpconn != NULL) { |
| 397 | client_read(client); |
| 398 | } else { |
| 399 | client_accept(client); |
| 400 | @@ -2470,6 +2454,7 @@ client_request(isc_task_t *task, isc_event_t *event) { |
| 401 | client->nrecvs--; |
| 402 | } else { |
| 403 | INSIST(TCP_CLIENT(client)); |
| 404 | + INSIST(client->tcpconn != NULL); |
| 405 | REQUIRE(event->ev_type == DNS_EVENT_TCPMSG); |
| 406 | REQUIRE(event->ev_sender == &client->tcpmsg); |
| 407 | buffer = &client->tcpmsg.buffer; |
| 408 | @@ -2657,17 +2642,19 @@ client_request(isc_task_t *task, isc_event_t *event) { |
| 409 | /* |
| 410 | * Pipeline TCP query processing. |
| 411 | */ |
| 412 | - if (client->message->opcode != dns_opcode_query) { |
| 413 | - client->pipelined = false; |
| 414 | + if (TCP_CLIENT(client) && |
| 415 | + client->message->opcode != dns_opcode_query) |
| 416 | + { |
| 417 | + client->tcpconn->pipelined = false; |
| 418 | } |
| 419 | - if (TCP_CLIENT(client) && client->pipelined) { |
| 420 | + if (TCP_CLIENT(client) && client->tcpconn->pipelined) { |
| 421 | /* |
| 422 | * We're pipelining. Replace the client; the |
| 423 | - * the replacement can read the TCP socket looking |
| 424 | - * for new messages and this client can process the |
| 425 | + * replacement can read the TCP socket looking |
| 426 | + * for new messages and this one can process the |
| 427 | * current message asynchronously. |
| 428 | * |
| 429 | - * There are now at least three clients using this |
| 430 | + * There will now be at least three clients using this |
| 431 | * TCP socket - one accepting new connections, |
| 432 | * one reading an existing connection to get new |
| 433 | * messages, and one answering the message already |
| 434 | @@ -2675,7 +2662,7 @@ client_request(isc_task_t *task, isc_event_t *event) { |
| 435 | */ |
| 436 | result = ns_client_replace(client); |
| 437 | if (result != ISC_R_SUCCESS) { |
| 438 | - client->pipelined = false; |
| 439 | + client->tcpconn->pipelined = false; |
| 440 | } |
| 441 | } |
| 442 | |
| 443 | @@ -3233,10 +3220,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { |
| 444 | client->signer = NULL; |
| 445 | dns_name_init(&client->signername, NULL); |
| 446 | client->mortal = false; |
| 447 | - client->pipelined = false; |
| 448 | - client->pipeline_refs = NULL; |
| 449 | - client->tcpquota = NULL; |
| 450 | - client->tcpattached = false; |
| 451 | + client->tcpconn = NULL; |
| 452 | client->recursionquota = NULL; |
| 453 | client->interface = NULL; |
| 454 | client->peeraddr_valid = false; |
| 455 | @@ -3341,9 +3325,10 @@ client_read(ns_client_t *client) { |
| 456 | |
| 457 | static void |
| 458 | client_newconn(isc_task_t *task, isc_event_t *event) { |
| 459 | + isc_result_t result; |
| 460 | ns_client_t *client = event->ev_arg; |
| 461 | isc_socket_newconnev_t *nevent = (isc_socket_newconnev_t *)event; |
| 462 | - isc_result_t result; |
| 463 | + uint32_t old; |
| 464 | |
| 465 | REQUIRE(event->ev_type == ISC_SOCKEVENT_NEWCONN); |
| 466 | REQUIRE(NS_CLIENT_VALID(client)); |
| 467 | @@ -3363,10 +3348,8 @@ client_newconn(isc_task_t *task, isc_event_t *event) { |
| 468 | INSIST(client->naccepts == 1); |
| 469 | client->naccepts--; |
| 470 | |
| 471 | - LOCK(&client->interface->lock); |
| 472 | - INSIST(client->interface->ntcpaccepting > 0); |
| 473 | - client->interface->ntcpaccepting--; |
| 474 | - UNLOCK(&client->interface->lock); |
| 475 | + old = isc_atomic_xadd(&client->interface->ntcpaccepting, -1); |
| 476 | + INSIST(old > 0); |
| 477 | |
| 478 | /* |
| 479 | * We must take ownership of the new socket before the exit |
| 480 | @@ -3399,7 +3382,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) { |
| 481 | NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), |
| 482 | "accept failed: %s", |
| 483 | isc_result_totext(nevent->result)); |
| 484 | - tcpquota_disconnect(client); |
| 485 | + tcpconn_detach(client); |
| 486 | } |
| 487 | |
| 488 | if (exit_check(client)) |
| 489 | @@ -3437,15 +3420,13 @@ client_newconn(isc_task_t *task, isc_event_t *event) { |
| 490 | * telnetting to port 53 (once per CPU) will |
| 491 | * deny service to legitimate TCP clients. |
| 492 | */ |
| 493 | - client->pipelined = false; |
| 494 | result = ns_client_replace(client); |
| 495 | if (result == ISC_R_SUCCESS && |
| 496 | (ns_g_server->keepresporder == NULL || |
| 497 | !allowed(&netaddr, NULL, NULL, 0, NULL, |
| 498 | ns_g_server->keepresporder))) |
| 499 | { |
| 500 | - pipeline_init(client); |
| 501 | - client->pipelined = true; |
| 502 | + client->tcpconn->pipelined = true; |
| 503 | } |
| 504 | |
| 505 | client_read(client); |
| 506 | @@ -3462,78 +3443,59 @@ client_accept(ns_client_t *client) { |
| 507 | CTRACE("accept"); |
| 508 | |
| 509 | /* |
| 510 | - * The tcpquota object can only be simultaneously referenced a |
| 511 | - * pre-defined number of times; this is configured by 'tcp-clients' |
| 512 | - * in named.conf. If we can't attach to it here, that means the TCP |
| 513 | - * client quota has been exceeded. |
| 514 | + * Set up a new TCP connection. This means try to attach to the |
| 515 | + * TCP client quota (tcp-clients), but fail if we're over quota. |
| 516 | */ |
| 517 | - result = isc_quota_attach(&ns_g_server->tcpquota, |
| 518 | - &client->tcpquota); |
| 519 | + result = tcpconn_init(client, false); |
| 520 | if (result != ISC_R_SUCCESS) { |
| 521 | - bool exit; |
| 522 | + bool exit; |
| 523 | |
| 524 | - ns_client_log(client, NS_LOGCATEGORY_CLIENT, |
| 525 | - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(1), |
| 526 | - "no more TCP clients: %s", |
| 527 | - isc_result_totext(result)); |
| 528 | - |
| 529 | - /* |
| 530 | - * We have exceeded the system-wide TCP client |
| 531 | - * quota. But, we can't just block this accept |
| 532 | - * in all cases, because if we did, a heavy TCP |
| 533 | - * load on other interfaces might cause this |
| 534 | - * interface to be starved, with no clients able |
| 535 | - * to accept new connections. |
| 536 | - * |
| 537 | - * So, we check here to see if any other clients |
| 538 | - * are already servicing TCP queries on this |
| 539 | - * interface (whether accepting, reading, or |
| 540 | - * processing). If there are at least two |
| 541 | - * (one reading and one processing a request) |
| 542 | - * then it's okay *not* to call accept - we |
| 543 | - * can let this client go inactive and another |
| 544 | - * one will resume accepting when it's done. |
| 545 | - * |
| 546 | - * If there aren't enough active clients on the |
| 547 | - * interface, then we can be a little bit |
| 548 | - * flexible about the quota. We'll allow *one* |
| 549 | - * extra client through to ensure we're listening |
| 550 | - * on every interface. |
| 551 | - * |
| 552 | - * (Note: In practice this means that the real |
| 553 | - * TCP client quota is tcp-clients plus the |
| 554 | - * number of listening interfaces plus 2.) |
| 555 | - */ |
| 556 | - LOCK(&client->interface->lock); |
| 557 | - exit = (client->interface->ntcpactive > 1); |
| 558 | - UNLOCK(&client->interface->lock); |
| 559 | + ns_client_log(client, NS_LOGCATEGORY_CLIENT, |
| 560 | + NS_LOGMODULE_CLIENT, ISC_LOG_WARNING, |
| 561 | + "TCP client quota reached: %s", |
| 562 | + isc_result_totext(result)); |
| 563 | |
| 564 | - if (exit) { |
| 565 | - client->newstate = NS_CLIENTSTATE_INACTIVE; |
| 566 | - (void)exit_check(client); |
| 567 | - return; |
| 568 | - } |
| 569 | + /* |
| 570 | + * We have exceeded the system-wide TCP client quota. But, |
| 571 | + * we can't just block this accept in all cases, because if |
| 572 | + * we did, a heavy TCP load on other interfaces might cause |
| 573 | + * this interface to be starved, with no clients able to |
| 574 | + * accept new connections. |
| 575 | + * |
| 576 | + * So, we check here to see if any other clients are |
| 577 | + * already servicing TCP queries on this interface (whether |
| 578 | + * accepting, reading, or processing). If we find at least |
| 579 | + * one, then it's okay *not* to call accept - we can let this |
| 580 | + * client go inactive and another will take over when it's |
| 581 | + * done. |
| 582 | + * |
| 583 | + * If there aren't enough active clients on the interface, |
| 584 | + * then we can be a little bit flexible about the quota. |
| 585 | + * We'll allow *one* extra client through to ensure we're |
| 586 | + * listening on every interface; we do this by setting the |
| 587 | + * 'force' option to tcpconn_init(). |
| 588 | + * |
| 589 | + * (Note: In practice this means that the real TCP client |
| 590 | + * quota is tcp-clients plus the number of listening |
| 591 | + * interfaces plus 1.) |
| 592 | + */ |
| 593 | + exit = (isc_atomic_xadd(&client->interface->ntcpactive, 0) > 0); |
| 594 | + if (exit) { |
| 595 | + client->newstate = NS_CLIENTSTATE_INACTIVE; |
| 596 | + (void)exit_check(client); |
| 597 | + return; |
| 598 | + } |
| 599 | |
| 600 | - } else { |
| 601 | - client->tcpattached = true; |
| 602 | + result = tcpconn_init(client, true); |
| 603 | + RUNTIME_CHECK(result == ISC_R_SUCCESS); |
| 604 | } |
| 605 | |
| 606 | /* |
| 607 | - * By incrementing the interface's ntcpactive counter we signal |
| 608 | - * that there is at least one client servicing TCP queries for the |
| 609 | - * interface. |
| 610 | - * |
| 611 | - * We also make note of the fact in the client itself with the |
| 612 | - * tcpactive flag. This ensures proper accounting by preventing |
| 613 | - * us from accidentally incrementing or decrementing ntcpactive |
| 614 | - * more than once per client object. |
| 615 | + * If this client was set up using get_client() or get_worker(), |
| 616 | + * then TCP is already marked active. However, if it was restarted |
| 617 | + * from exit_check(), it might not be, so we take care of it now. |
| 618 | */ |
| 619 | - if (!client->tcpactive) { |
| 620 | - LOCK(&client->interface->lock); |
| 621 | - client->interface->ntcpactive++; |
| 622 | - UNLOCK(&client->interface->lock); |
| 623 | - client->tcpactive = true; |
| 624 | - } |
| 625 | + mark_tcp_active(client, true); |
| 626 | |
| 627 | result = isc_socket_accept(client->tcplistener, client->task, |
| 628 | client_newconn, client); |
| 629 | @@ -3549,15 +3511,8 @@ client_accept(ns_client_t *client) { |
| 630 | "isc_socket_accept() failed: %s", |
| 631 | isc_result_totext(result)); |
| 632 | |
| 633 | - tcpquota_disconnect(client); |
| 634 | - |
| 635 | - if (client->tcpactive) { |
| 636 | - LOCK(&client->interface->lock); |
| 637 | - client->interface->ntcpactive--; |
| 638 | - UNLOCK(&client->interface->lock); |
| 639 | - client->tcpactive = false; |
| 640 | - } |
| 641 | - |
| 642 | + tcpconn_detach(client); |
| 643 | + mark_tcp_active(client, false); |
| 644 | return; |
| 645 | } |
| 646 | |
| 647 | @@ -3582,9 +3537,7 @@ client_accept(ns_client_t *client) { |
| 648 | * listening for connections itself to prevent the interface |
| 649 | * going dead. |
| 650 | */ |
| 651 | - LOCK(&client->interface->lock); |
| 652 | - client->interface->ntcpaccepting++; |
| 653 | - UNLOCK(&client->interface->lock); |
| 654 | + isc_atomic_xadd(&client->interface->ntcpaccepting, 1); |
| 655 | } |
| 656 | |
| 657 | static void |
| 658 | @@ -3655,24 +3608,25 @@ ns_client_replace(ns_client_t *client) { |
| 659 | REQUIRE(client->manager != NULL); |
| 660 | |
| 661 | tcp = TCP_CLIENT(client); |
| 662 | - if (tcp && client->pipelined) { |
| 663 | + if (tcp && client->tcpconn != NULL && client->tcpconn->pipelined) { |
| 664 | result = get_worker(client->manager, client->interface, |
| 665 | client->tcpsocket, client); |
| 666 | } else { |
| 667 | result = get_client(client->manager, client->interface, |
| 668 | - client->dispatch, client, tcp); |
| 669 | + client->dispatch, tcp); |
| 670 | |
| 671 | - /* |
| 672 | - * The responsibility for listening for new requests is hereby |
| 673 | - * transferred to the new client. Therefore, the old client |
| 674 | - * should refrain from listening for any more requests. |
| 675 | - */ |
| 676 | - client->mortal = true; |
| 677 | } |
| 678 | if (result != ISC_R_SUCCESS) { |
| 679 | return (result); |
| 680 | } |
| 681 | |
| 682 | + /* |
| 683 | + * The responsibility for listening for new requests is hereby |
| 684 | + * transferred to the new client. Therefore, the old client |
| 685 | + * should refrain from listening for any more requests. |
| 686 | + */ |
| 687 | + client->mortal = true; |
| 688 | + |
| 689 | return (ISC_R_SUCCESS); |
| 690 | } |
| 691 | |
| 692 | @@ -3806,7 +3760,7 @@ ns_clientmgr_destroy(ns_clientmgr_t **managerp) { |
| 693 | |
| 694 | static isc_result_t |
| 695 | get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, |
| 696 | - dns_dispatch_t *disp, ns_client_t *oldclient, bool tcp) |
| 697 | + dns_dispatch_t *disp, bool tcp) |
| 698 | { |
| 699 | isc_result_t result = ISC_R_SUCCESS; |
| 700 | isc_event_t *ev; |
| 701 | @@ -3850,15 +3804,7 @@ get_client(ns_clientmgr_t *manager, ns_interface_t *ifp, |
| 702 | client->dscp = ifp->dscp; |
| 703 | |
| 704 | if (tcp) { |
| 705 | - client->tcpattached = false; |
| 706 | - if (oldclient != NULL) { |
| 707 | - client->tcpattached = oldclient->tcpattached; |
| 708 | - } |
| 709 | - |
| 710 | - LOCK(&client->interface->lock); |
| 711 | - client->interface->ntcpactive++; |
| 712 | - UNLOCK(&client->interface->lock); |
| 713 | - client->tcpactive = true; |
| 714 | + mark_tcp_active(client, true); |
| 715 | |
| 716 | client->attributes |= NS_CLIENTATTR_TCP; |
| 717 | isc_socket_attach(ifp->tcpsocket, |
| 718 | @@ -3923,16 +3869,14 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock, |
| 719 | ns_interface_attach(ifp, &client->interface); |
| 720 | client->newstate = client->state = NS_CLIENTSTATE_WORKING; |
| 721 | INSIST(client->recursionquota == NULL); |
| 722 | - client->tcpquota = &ns_g_server->tcpquota; |
| 723 | - client->tcpattached = oldclient->tcpattached; |
| 724 | |
| 725 | client->dscp = ifp->dscp; |
| 726 | |
| 727 | client->attributes |= NS_CLIENTATTR_TCP; |
| 728 | client->mortal = true; |
| 729 | |
| 730 | - pipeline_attach(oldclient, client); |
| 731 | - client->pipelined = true; |
| 732 | + tcpconn_attach(oldclient, client); |
| 733 | + mark_tcp_active(client, true); |
| 734 | |
| 735 | isc_socket_attach(ifp->tcpsocket, &client->tcplistener); |
| 736 | isc_socket_attach(sock, &client->tcpsocket); |
| 737 | @@ -3940,11 +3884,6 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock, |
| 738 | (void)isc_socket_getpeername(client->tcpsocket, &client->peeraddr); |
| 739 | client->peeraddr_valid = true; |
| 740 | |
| 741 | - LOCK(&client->interface->lock); |
| 742 | - client->interface->ntcpactive++; |
| 743 | - UNLOCK(&client->interface->lock); |
| 744 | - client->tcpactive = true; |
| 745 | - |
| 746 | INSIST(client->tcpmsg_valid == false); |
| 747 | dns_tcpmsg_init(client->mctx, client->tcpsocket, &client->tcpmsg); |
| 748 | client->tcpmsg_valid = true; |
| 749 | @@ -3970,8 +3909,7 @@ ns_clientmgr_createclients(ns_clientmgr_t *manager, unsigned int n, |
| 750 | MTRACE("createclients"); |
| 751 | |
| 752 | for (disp = 0; disp < n; disp++) { |
| 753 | - result = get_client(manager, ifp, ifp->udpdispatch[disp], |
| 754 | - NULL, tcp); |
| 755 | + result = get_client(manager, ifp, ifp->udpdispatch[disp], tcp); |
| 756 | if (result != ISC_R_SUCCESS) |
| 757 | break; |
| 758 | } |
| 759 | diff --git a/bin/named/include/named/client.h b/bin/named/include/named/client.h |
| 760 | index e2c40acd28..969ee4c08f 100644 |
| 761 | --- a/bin/named/include/named/client.h |
| 762 | +++ b/bin/named/include/named/client.h |
| 763 | @@ -78,6 +78,13 @@ |
| 764 | *** Types |
| 765 | ***/ |
| 766 | |
| 767 | +/*% reference-counted TCP connection object */ |
| 768 | +typedef struct ns_tcpconn { |
| 769 | + isc_refcount_t refs; |
| 770 | + isc_quota_t *tcpquota; |
| 771 | + bool pipelined; |
| 772 | +} ns_tcpconn_t; |
| 773 | + |
| 774 | /*% nameserver client structure */ |
| 775 | struct ns_client { |
| 776 | unsigned int magic; |
| 777 | @@ -131,10 +138,7 @@ struct ns_client { |
| 778 | dns_name_t signername; /*%< [T]SIG key name */ |
| 779 | dns_name_t *signer; /*%< NULL if not valid sig */ |
| 780 | bool mortal; /*%< Die after handling request */ |
| 781 | - bool pipelined; /*%< TCP queries not in sequence */ |
| 782 | - isc_refcount_t *pipeline_refs; |
| 783 | - isc_quota_t *tcpquota; |
| 784 | - bool tcpattached; |
| 785 | + ns_tcpconn_t *tcpconn; |
| 786 | isc_quota_t *recursionquota; |
| 787 | ns_interface_t *interface; |
| 788 | |
| 789 | diff --git a/bin/named/include/named/interfacemgr.h b/bin/named/include/named/interfacemgr.h |
| 790 | index 61b08826a6..3535ef22a8 100644 |
| 791 | --- a/bin/named/include/named/interfacemgr.h |
| 792 | +++ b/bin/named/include/named/interfacemgr.h |
| 793 | @@ -9,8 +9,6 @@ |
| 794 | * information regarding copyright ownership. |
| 795 | */ |
| 796 | |
| 797 | -/* $Id: interfacemgr.h,v 1.35 2011/07/28 23:47:58 tbox Exp $ */ |
| 798 | - |
| 799 | #ifndef NAMED_INTERFACEMGR_H |
| 800 | #define NAMED_INTERFACEMGR_H 1 |
| 801 | |
| 802 | @@ -77,11 +75,11 @@ struct ns_interface { |
| 803 | /*%< UDP dispatchers. */ |
| 804 | isc_socket_t * tcpsocket; /*%< TCP socket. */ |
| 805 | isc_dscp_t dscp; /*%< "listen-on" DSCP value */ |
| 806 | - int ntcpaccepting; /*%< Number of clients |
| 807 | + int32_t ntcpaccepting; /*%< Number of clients |
| 808 | ready to accept new |
| 809 | TCP connections on this |
| 810 | interface */ |
| 811 | - int ntcpactive; /*%< Number of clients |
| 812 | + int32_t ntcpactive; /*%< Number of clients |
| 813 | servicing TCP queries |
| 814 | (whether accepting or |
| 815 | connected) */ |
| 816 | diff --git a/bin/named/interfacemgr.c b/bin/named/interfacemgr.c |
| 817 | index 955096ef47..d9f6df5802 100644 |
| 818 | --- a/bin/named/interfacemgr.c |
| 819 | +++ b/bin/named/interfacemgr.c |
| 820 | @@ -388,6 +388,7 @@ ns_interface_create(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr, |
| 821 | */ |
| 822 | ifp->ntcpaccepting = 0; |
| 823 | ifp->ntcpactive = 0; |
| 824 | + |
| 825 | ifp->nudpdispatch = 0; |
| 826 | |
| 827 | ifp->dscp = -1; |
| 828 | diff --git a/lib/isc/include/isc/quota.h b/lib/isc/include/isc/quota.h |
| 829 | index b9bf59877a..36c5830242 100644 |
| 830 | --- a/lib/isc/include/isc/quota.h |
| 831 | +++ b/lib/isc/include/isc/quota.h |
| 832 | @@ -100,6 +100,13 @@ isc_quota_attach(isc_quota_t *quota, isc_quota_t **p); |
| 833 | * quota if successful (ISC_R_SUCCESS or ISC_R_SOFTQUOTA). |
| 834 | */ |
| 835 | |
| 836 | +isc_result_t |
| 837 | +isc_quota_force(isc_quota_t *quota, isc_quota_t **p); |
| 838 | +/*%< |
| 839 | + * Like isc_quota_attach, but will attach '*p' to the quota |
| 840 | + * even if the hard quota has been exceeded. |
| 841 | + */ |
| 842 | + |
| 843 | void |
| 844 | isc_quota_detach(isc_quota_t **p); |
| 845 | /*%< |
| 846 | diff --git a/lib/isc/quota.c b/lib/isc/quota.c |
| 847 | index 3ddff0d875..556a61f21d 100644 |
| 848 | --- a/lib/isc/quota.c |
| 849 | +++ b/lib/isc/quota.c |
| 850 | @@ -74,20 +74,39 @@ isc_quota_release(isc_quota_t *quota) { |
| 851 | UNLOCK("a->lock); |
| 852 | } |
| 853 | |
| 854 | -isc_result_t |
| 855 | -isc_quota_attach(isc_quota_t *quota, isc_quota_t **p) |
| 856 | -{ |
| 857 | +static isc_result_t |
| 858 | +doattach(isc_quota_t *quota, isc_quota_t **p, bool force) { |
| 859 | isc_result_t result; |
| 860 | - INSIST(p != NULL && *p == NULL); |
| 861 | + REQUIRE(p != NULL && *p == NULL); |
| 862 | + |
| 863 | result = isc_quota_reserve(quota); |
| 864 | - if (result == ISC_R_SUCCESS || result == ISC_R_SOFTQUOTA) |
| 865 | + if (result == ISC_R_SUCCESS || result == ISC_R_SOFTQUOTA) { |
| 866 | + *p = quota; |
| 867 | + } else if (result == ISC_R_QUOTA && force) { |
| 868 | + /* attach anyway */ |
| 869 | + LOCK("a->lock); |
| 870 | + quota->used++; |
| 871 | + UNLOCK("a->lock); |
| 872 | + |
| 873 | *p = quota; |
| 874 | + result = ISC_R_SUCCESS; |
| 875 | + } |
| 876 | + |
| 877 | return (result); |
| 878 | } |
| 879 | |
| 880 | +isc_result_t |
| 881 | +isc_quota_attach(isc_quota_t *quota, isc_quota_t **p) { |
| 882 | + return (doattach(quota, p, false)); |
| 883 | +} |
| 884 | + |
| 885 | +isc_result_t |
| 886 | +isc_quota_force(isc_quota_t *quota, isc_quota_t **p) { |
| 887 | + return (doattach(quota, p, true)); |
| 888 | +} |
| 889 | + |
| 890 | void |
| 891 | -isc_quota_detach(isc_quota_t **p) |
| 892 | -{ |
| 893 | +isc_quota_detach(isc_quota_t **p) { |
| 894 | INSIST(p != NULL && *p != NULL); |
| 895 | isc_quota_release(*p); |
| 896 | *p = NULL; |
| 897 | diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in |
| 898 | index a82facec0f..7b9f23d776 100644 |
| 899 | --- a/lib/isc/win32/libisc.def.in |
| 900 | +++ b/lib/isc/win32/libisc.def.in |
| 901 | @@ -519,6 +519,7 @@ isc_portset_removerange |
| 902 | isc_quota_attach |
| 903 | isc_quota_destroy |
| 904 | isc_quota_detach |
| 905 | +isc_quota_force |
| 906 | isc_quota_init |
| 907 | isc_quota_max |
| 908 | isc_quota_release |
| 909 | -- |
| 910 | 2.20.1 |
| 911 | |