details: http://hg.nginx.org/nginx/rev/f63dd04c1580
branches: stable-1.8
changeset: 6356:f63dd04c1580
user: Ruslan Ermilov <ru@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
@@ -422,7 +422,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);

@@ -576,6 +576,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;
@@ -669,9 +683,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);
}
@@ -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

/* 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;
-
- 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;


_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel