details: http://hg.nginx.org/nginx/rev/e662cbf1b932
branches:
changeset: 6926:e662cbf1b932
user: Maxim Dounin <mdounin@mdounin.ru>
date: Tue Mar 07 18:49:31 2017 +0300
description:
Converted hc->busy/hc->free to use chain links.

Most notably, this fixes possible buffer overflows if number of large
client header buffers in a virtual server is different from the one in
the default server.

Reported by Daniil Bondarev.

diffstat:

src/http/ngx_http_request.c | 91 ++++++++++++++++++++++++++------------------
src/http/ngx_http_request.h | 5 +-
2 files changed, 55 insertions(+), 41 deletions(-)

diffs (194 lines):

diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -549,7 +549,7 @@ ngx_http_create_request(ngx_connection_t

ngx_set_connection_log(r->connection, clcf->error_log);

- r->header_in = hc->nbusy ? hc->busy[0] : c->buffer;
+ r->header_in = hc->busy ? hc->busy->buf : c->buffer;

if (ngx_list_init(&r->headers_out.headers, r->pool, 20,
sizeof(ngx_table_elt_t))
@@ -1431,6 +1431,7 @@ ngx_http_alloc_large_header_buffer(ngx_h
{
u_char *old, *new;
ngx_buf_t *b;
+ ngx_chain_t *cl;
ngx_http_connection_t *hc;
ngx_http_core_srv_conf_t *cscf;

@@ -1460,8 +1461,11 @@ ngx_http_alloc_large_header_buffer(ngx_h

hc = r->http_connection;

- if (hc->nfree) {
- b = hc->free[--hc->nfree];
+ if (hc->free) {
+ cl = hc->free;
+ hc->free = cl->next;
+
+ b = cl->buf;

ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"http large header free: %p %uz",
@@ -1469,20 +1473,19 @@ ngx_http_alloc_large_header_buffer(ngx_h

} else if (hc->nbusy < cscf->large_client_header_buffers.num) {

- if (hc->busy == NULL) {
- hc->busy = ngx_palloc(r->connection->pool,
- cscf->large_client_header_buffers.num * sizeof(ngx_buf_t *));
- if (hc->busy == NULL) {
- return NGX_ERROR;
- }
- }
-
b = ngx_create_temp_buf(r->connection->pool,
cscf->large_client_header_buffers.size);
if (b == NULL) {
return NGX_ERROR;
}

+ cl = ngx_alloc_chain_link(r->connection->pool);
+ if (cl == NULL) {
+ return NGX_ERROR;
+ }
+
+ cl->buf = b;
+
ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"http large header alloc: %p %uz",
b->pos, b->end - b->last);
@@ -1491,7 +1494,9 @@ ngx_http_alloc_large_header_buffer(ngx_h
return NGX_DECLINED;
}

- hc->busy[hc->nbusy++] = b;
+ cl->next = hc->busy;
+ hc->busy = cl;
+ hc->nbusy++;

if (r->state == 0) {
/*
@@ -2835,12 +2840,11 @@ static void
ngx_http_set_keepalive(ngx_http_request_t *r)
{
int tcp_nodelay;
- ngx_int_t i;
ngx_buf_t *b, *f;
+ ngx_chain_t *cl, *ln;
ngx_event_t *rev, *wev;
ngx_connection_t *c;
ngx_http_connection_t *hc;
- ngx_http_core_srv_conf_t *cscf;
ngx_http_core_loc_conf_t *clcf;

c = r->connection;
@@ -2876,26 +2880,32 @@ ngx_http_set_keepalive(ngx_http_request_
* Now we would move the large header buffers to the free list.
*/

- cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
-
- if (hc->free == NULL) {
- hc->free = ngx_palloc(c->pool,
- cscf->large_client_header_buffers.num * sizeof(ngx_buf_t *));
-
- if (hc->free == NULL) {
- ngx_http_close_request(r, 0);
- return;
+ for (cl = hc->busy; cl; /* void */) {
+ ln = cl;
+ cl = cl->next;
+
+ if (ln->buf == b) {
+ ngx_free_chain(c->pool, ln);
+ continue;
}
- }
-
- for (i = 0; i < hc->nbusy - 1; i++) {
- f = hc->busy[i];
- hc->free[hc->nfree++] = f;
+
+ f = ln->buf;
f->pos = f->start;
f->last = f->start;
+
+ ln->next = hc->free;
+ hc->free = ln;
}

- hc->busy[0] = b;
+ cl = ngx_alloc_chain_link(c->pool);
+ if (cl == NULL) {
+ ngx_http_close_request(r, 0);
+ return;
+ }
+
+ cl->buf = b;
+
+ hc->busy = cl;
hc->nbusy = 1;
}
}
@@ -2966,27 +2976,32 @@ ngx_http_set_keepalive(ngx_http_request_
b->last = b->start;
}

- ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0, "hc free: %p %i",
- hc->free, hc->nfree);
+ ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, "hc free: %p",
+ hc->free);

if (hc->free) {
- for (i = 0; i < hc->nfree; i++) {
- ngx_pfree(c->pool, hc->free[i]->start);
- hc->free[i] = NULL;
+ for (cl = hc->free; cl; /* void */) {
+ ln = cl;
+ cl = cl->next;
+ ngx_pfree(c->pool, ln->buf->start);
+ ngx_free_chain(c->pool, ln);
}

- hc->nfree = 0;
+ hc->free = NULL;
}

ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0, "hc busy: %p %i",
hc->busy, hc->nbusy);

if (hc->busy) {
- for (i = 0; i < hc->nbusy; i++) {
- ngx_pfree(c->pool, hc->busy[i]->start);
- hc->busy[i] = NULL;
+ for (cl = hc->busy; cl; /* void */) {
+ ln = cl;
+ cl = cl->next;
+ ngx_pfree(c->pool, ln->buf->start);
+ ngx_free_chain(c->pool, ln);
}

+ hc->busy = NULL;
hc->nbusy = 0;
}

diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -309,11 +309,10 @@ typedef struct {
#endif
#endif

- ngx_buf_t **busy;
+ ngx_chain_t *busy;
ngx_int_t nbusy;

- ngx_buf_t **free;
- ngx_int_t nfree;
+ ngx_chain_t *free;

unsigned ssl:1;
unsigned proxy_protocol:1;
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel