Mercurial > hg > nginx

changeset 7213:c69c13f10502

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Geo: fixed memory allocation error handling (closes #1482). If during configuration parsing of the geo directive the memory allocation has failed, pool used to parse configuration inside the block, and sometimes the temporary pool were not destroyed.
author Ruslan Ermilov <ru@nginx.com>
date Wed, 21 Feb 2018 15:50:42 +0300
parents 0237af43d409
children 88aad69eccef
files src/http/modules/ngx_http_geo_module.c src/stream/ngx_stream_geo_module.c
diffstat 2 files changed, 32 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/src/http/modules/ngx_http_geo_module.c
+++ b/src/http/modules/ngx_http_geo_module.c
@@ -439,6 +439,7 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
 
     ctx.temp_pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, cf->log);
     if (ctx.temp_pool == NULL) {
+        ngx_destroy_pool(pool);
         return NGX_CONF_ERROR;
     }
 
@@ -482,7 +483,7 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
 
                 ctx.high.low[i] = ngx_palloc(cf->pool, len + sizeof(void *));
                 if (ctx.high.low[i] == NULL) {
-                    return NGX_CONF_ERROR;
+                    goto failed;
                 }
 
                 ngx_memcpy(ctx.high.low[i], a->elts, len);
@@ -508,14 +509,11 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
         var->get_handler = ngx_http_geo_range_variable;
         var->data = (uintptr_t) geo;
 
-        ngx_destroy_pool(ctx.temp_pool);
-        ngx_destroy_pool(pool);
-
     } else {
         if (ctx.tree == NULL) {
             ctx.tree = ngx_radix_tree_create(cf->pool, -1);
             if (ctx.tree == NULL) {
-                return NGX_CONF_ERROR;
+                goto failed;
             }
         }
 
@@ -525,7 +523,7 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
         if (ctx.tree6 == NULL) {
             ctx.tree6 = ngx_radix_tree_create(cf->pool, -1);
             if (ctx.tree6 == NULL) {
-                return NGX_CONF_ERROR;
+                goto failed;
             }
         }
 
@@ -535,14 +533,11 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
         var->get_handler = ngx_http_geo_cidr_variable;
         var->data = (uintptr_t) geo;
 
-        ngx_destroy_pool(ctx.temp_pool);
-        ngx_destroy_pool(pool);
-
         if (ngx_radix32tree_insert(ctx.tree, 0, 0,
                                    (uintptr_t) &ngx_http_variable_null_value)
             == NGX_ERROR)
         {
-            return NGX_CONF_ERROR;
+            goto failed;
         }
 
         /* NGX_BUSY is okay (default was set explicitly) */
@@ -552,12 +547,22 @@ ngx_http_geo_block(ngx_conf_t *cf, ngx_c
                                     (uintptr_t) &ngx_http_variable_null_value)
             == NGX_ERROR)
         {
-            return NGX_CONF_ERROR;
+            goto failed;
         }
 #endif
     }
 
+    ngx_destroy_pool(ctx.temp_pool);
+    ngx_destroy_pool(pool);
+
     return rv;
+
+failed:
+
+    ngx_destroy_pool(ctx.temp_pool);
+    ngx_destroy_pool(pool);
+
+    return NGX_CONF_ERROR;
 }
 
 
--- a/src/stream/ngx_stream_geo_module.c
+++ b/src/stream/ngx_stream_geo_module.c
@@ -409,6 +409,7 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
 
     ctx.temp_pool = ngx_create_pool(NGX_DEFAULT_POOL_SIZE, cf->log);
     if (ctx.temp_pool == NULL) {
+        ngx_destroy_pool(pool);
         return NGX_CONF_ERROR;
     }
 
@@ -449,7 +450,7 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
 
                 ctx.high.low[i] = ngx_palloc(cf->pool, len + sizeof(void *));
                 if (ctx.high.low[i] == NULL) {
-                    return NGX_CONF_ERROR;
+                    goto failed;
                 }
 
                 ngx_memcpy(ctx.high.low[i], a->elts, len);
@@ -475,14 +476,11 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
         var->get_handler = ngx_stream_geo_range_variable;
         var->data = (uintptr_t) geo;
 
-        ngx_destroy_pool(ctx.temp_pool);
-        ngx_destroy_pool(pool);
-
     } else {
         if (ctx.tree == NULL) {
             ctx.tree = ngx_radix_tree_create(cf->pool, -1);
             if (ctx.tree == NULL) {
-                return NGX_CONF_ERROR;
+                goto failed;
             }
         }
 
@@ -492,7 +490,7 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
         if (ctx.tree6 == NULL) {
             ctx.tree6 = ngx_radix_tree_create(cf->pool, -1);
             if (ctx.tree6 == NULL) {
-                return NGX_CONF_ERROR;
+                goto failed;
             }
         }
 
@@ -502,14 +500,11 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
         var->get_handler = ngx_stream_geo_cidr_variable;
         var->data = (uintptr_t) geo;
 
-        ngx_destroy_pool(ctx.temp_pool);
-        ngx_destroy_pool(pool);
-
         if (ngx_radix32tree_insert(ctx.tree, 0, 0,
                                    (uintptr_t) &ngx_stream_variable_null_value)
             == NGX_ERROR)
         {
-            return NGX_CONF_ERROR;
+            goto failed;
         }
 
         /* NGX_BUSY is okay (default was set explicitly) */
@@ -519,12 +514,22 @@ ngx_stream_geo_block(ngx_conf_t *cf, ngx
                                     (uintptr_t) &ngx_stream_variable_null_value)
             == NGX_ERROR)
         {
-            return NGX_CONF_ERROR;
+            goto failed;
         }
 #endif
     }
 
+    ngx_destroy_pool(ctx.temp_pool);
+    ngx_destroy_pool(pool);
+
     return rv;
+
+failed:
+
+    ngx_destroy_pool(ctx.temp_pool);
+    ngx_destroy_pool(pool);
+
+    return NGX_CONF_ERROR;
 }