diff options
author | Ruslan Ermilov <ru@nginx.com> | 2016-01-26 16:46:31 +0300 |
---|---|---|
committer | Ruslan Ermilov <ru@nginx.com> | 2016-01-26 16:46:31 +0300 |
commit | 84ae282f938068fa5c09f7c157ea0086b2ab760b (patch) | |
tree | 8f778791f9d4d41542841c58a1b3613c3d012177 | |
parent | 717f65a5072d67e851c5e2f755eb7179a343c112 (diff) | |
download | nginx-84ae282f938068fa5c09f7c157ea0086b2ab760b.tar.gz |
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.
-rw-r--r-- | src/core/ngx_resolver.c | 54 | ||||
-rw-r--r-- | src/core/ngx_resolver.h | 13 |
2 files changed, 42 insertions, 25 deletions
diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index fceeed0af..642a983c6 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -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->handler(ctx); + ctx->state = NGX_RESOLVE_TIMEDOUT; - ctx = next; - } while (ctx); + ctx->handler(ctx); } diff --git a/src/core/ngx_resolver.h b/src/core/ngx_resolver.h index d3519fb6f..22f249595 100644 --- a/src/core/ngx_resolver.h +++ b/src/core/ngx_resolver.h @@ -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; |