[nginx] Resolver: fixed crashes in timeout handler.

[Maxim Dounin]

details:   http://hg.nginx.org/nginx/rev/7316c57e4fe7
branches:  
changeset: 6348:7316c57e4fe7
user:      Ruslan Ermilov <ru at nginx.com>
date:      Tue Jan 26 16:46:31 2016 +0300
description:
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.

diffstat:

 src/core/ngx_resolver.c |  60 +++++++++++++++++++++++++++++++-----------------
 src/core/ngx_resolver.h |  13 ++++-----
 2 files changed, 45 insertions(+), 28 deletions(-)

diffs (147 lines):

diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
--- a/src/core/ngx_resolver.c
+++ b/src/core/ngx_resolver.c
@@ -423,7 +423,7 @@ ngx_resolve_name_done(ngx_resolver_ctx_t
 
     /* 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 @@ ngx_resolve_name_locked(ngx_resolver_t *
 
         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 @@ ngx_resolve_name_locked(ngx_resolver_t *
         }
 
         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 @@ 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;
@@ -867,9 +893,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);
 
@@ -959,7 +985,7 @@ ngx_resolve_addr_done(ngx_resolver_ctx_t
 
     /* 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 @@ done:
 static void
 ngx_resolver_timeout_handler(ngx_event_t *ev)
 {
-    ngx_resolver_ctx_t   *ctx, *next;
-    ngx_resolver_node_t  *rn;
-
-    rn = ev->data;
-    ctx = rn->waiting;
-    rn->waiting = NULL;
-
-    do {
-        ctx->state = NGX_RESOLVE_TIMEDOUT;
-        next = ctx->next;
-
-        ctx->handler(ctx);
-
-        ctx = next;
-    } while (ctx);
+    ngx_resolver_ctx_t  *ctx;
+
+    ctx = ev->data;
+
+    ctx->state = NGX_RESOLVE_TIMEDOUT;
+
+    ctx->handler(ctx);
 }
 
 
diff --git a/src/core/ngx_resolver.h b/src/core/ngx_resolver.h
--- a/src/core/ngx_resolver.h
+++ b/src/core/ngx_resolver.h
@@ -51,16 +51,12 @@ typedef void (*ngx_resolver_handler_pt)(
 
 
 typedef struct {
+    ngx_rbtree_node_t         node;
+    ngx_queue_t               queue;
+
     /* PTR: resolved name, A: name to resolve */
     u_char                   *name;
 
-    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;
-
 #if (NGX_HAVE_INET6)
     /* PTR: IPv6 address to resolve (IPv4 address is in rbtree node key) */
     struct in6_addr           addr6;
@@ -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;