Resolver: fixed crashes in timeout handler.
If one or more requests were waiting for a response, then after
getting a CNAME response, the timeout event on the first request
remained active, pointing to the wrong node with an empty
rn->waiting list, and that could cause either null pointer
dereference or use-after-free memory access if this timeout
expired.
If several requests were waiting for a response, and the first
request terminated (e.g., due to client closing a connection),
other requests were left without a timeout and could potentially
wait indefinitely.
This is fixed by introducing per-request independent timeouts.
This change also reverts 954867a2f0a6 and 5004210e8c78.
diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
index 14bd2b3..c3af0ba 100644
--- a/src/core/ngx_resolver.c
+++ b/src/core/ngx_resolver.c
@@ -423,7 +423,7 @@
/* lock name mutex */
- if (ctx->state == NGX_AGAIN) {
+ if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) {
hash = ngx_crc32_short(ctx->name.data, ctx->name.len);
@@ -581,6 +581,20 @@
if (rn->waiting) {
+ if (ctx->event == NULL) {
+ ctx->event = ngx_resolver_calloc(r, sizeof(ngx_event_t));
+ if (ctx->event == NULL) {
+ return NGX_ERROR;
+ }
+
+ ctx->event->handler = ngx_resolver_timeout_handler;
+ ctx->event->data = ctx;
+ ctx->event->log = r->log;
+ ctx->ident = -1;
+
+ ngx_add_timer(ctx->event, ctx->timeout);
+ }
+
ctx->next = rn->waiting;
rn->waiting = ctx;
ctx->state = NGX_AGAIN;
@@ -674,9 +688,9 @@
}
ctx->event->handler = ngx_resolver_timeout_handler;
- ctx->event->data = rn;
+ ctx->event->data = ctx;
ctx->event->log = r->log;
- rn->ident = -1;
+ ctx->ident = -1;
ngx_add_timer(ctx->event, ctx->timeout);
}
@@ -804,6 +818,18 @@
if (rn->waiting) {
+ ctx->event = ngx_resolver_calloc(r, sizeof(ngx_event_t));
+ if (ctx->event == NULL) {
+ return NGX_ERROR;
+ }
+
+ ctx->event->handler = ngx_resolver_timeout_handler;
+ ctx->event->data = ctx;
+ ctx->event->log = r->log;
+ ctx->ident = -1;
+
+ ngx_add_timer(ctx->event, ctx->timeout);
+
ctx->next = rn->waiting;
rn->waiting = ctx;
ctx->state = NGX_AGAIN;
@@ -867,9 +893,9 @@
}
ctx->event->handler = ngx_resolver_timeout_handler;
- ctx->event->data = rn;
+ ctx->event->data = ctx;
ctx->event->log = r->log;
- rn->ident = -1;
+ ctx->ident = -1;
ngx_add_timer(ctx->event, ctx->timeout);
@@ -959,7 +985,7 @@
/* lock addr mutex */
- if (ctx->state == NGX_AGAIN) {
+ if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) {
switch (ctx->addr.sockaddr->sa_family) {
@@ -2815,21 +2841,13 @@
static void
ngx_resolver_timeout_handler(ngx_event_t *ev)
{
- ngx_resolver_ctx_t *ctx, *next;
- ngx_resolver_node_t *rn;
+ ngx_resolver_ctx_t *ctx;
- rn = ev->data;
- ctx = rn->waiting;
- rn->waiting = NULL;
+ ctx = ev->data;
- do {
- ctx->state = NGX_RESOLVE_TIMEDOUT;
- next = ctx->next;
+ ctx->state = NGX_RESOLVE_TIMEDOUT;
- ctx->handler(ctx);
-
- ctx = next;
- } while (ctx);
+ ctx->handler(ctx);
}
