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.
This commit is contained in:
Ruslan Ermilov 2016-01-26 16:46:31 +03:00
parent 824eae78fb
commit 92c27f267f
2 changed files with 42 additions and 25 deletions

View File

@ -422,7 +422,7 @@ ngx_resolve_name_done(ngx_resolver_ctx_t *ctx)
/* 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);
@ -576,6 +576,20 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx)
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;
@ -669,9 +683,9 @@ ngx_resolve_name_locked(ngx_resolver_t *r, ngx_resolver_ctx_t *ctx)
}
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);
}
@ -799,6 +813,18 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx)
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;
@ -862,9 +888,9 @@ ngx_resolve_addr(ngx_resolver_ctx_t *ctx)
}
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);
@ -954,7 +980,7 @@ ngx_resolve_addr_done(ngx_resolver_ctx_t *ctx)
/* lock addr mutex */
if (ctx->state == NGX_AGAIN) {
if (ctx->state == NGX_AGAIN || ctx->state == NGX_RESOLVE_TIMEDOUT) {
switch (ctx->addr.sockaddr->sa_family) {
@ -2795,21 +2821,13 @@ done:
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);
}

View File

@ -51,15 +51,11 @@ typedef void (*ngx_resolver_handler_pt)(ngx_resolver_ctx_t *ctx);
typedef struct {
/* PTR: resolved name, A: name to resolve */
u_char *name;
ngx_rbtree_node_t node;
ngx_queue_t queue;
/* event ident must be after 3 pointers as in ngx_connection_t */
ngx_int_t ident;
ngx_rbtree_node_t node;
/* PTR: resolved name, A: name to resolve */
u_char *name;
#if (NGX_HAVE_INET6)
/* PTR: IPv6 address to resolve (IPv4 address is in rbtree node key) */
@ -147,6 +143,9 @@ struct ngx_resolver_ctx_s {
ngx_resolver_t *resolver;
ngx_udp_connection_t *udp_connection;
/* event ident must be after 3 pointers as in ngx_connection_t */
ngx_int_t ident;
ngx_int_t state;
ngx_str_t name;