summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRuslan Ermilov <ru@nginx.com>2016-01-26 16:46:31 +0300
committerRuslan Ermilov <ru@nginx.com>2016-01-26 16:46:31 +0300
commit84ae282f938068fa5c09f7c157ea0086b2ab760b (patch)
tree8f778791f9d4d41542841c58a1b3613c3d012177
parent717f65a5072d67e851c5e2f755eb7179a343c112 (diff)
downloadnginx-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.c54
-rw-r--r--src/core/ngx_resolver.h13
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;