[nginx] Geo: fixed memory allocation error handling (closes #1482).

[Ruslan Ermilov]

details:   http://hg.nginx.org/nginx/rev/c69c13f10502
branches:  
changeset: 7213:c69c13f10502
user:      Ruslan Ermilov <ru at nginx.com>
date:      Wed Feb 21 15:50:42 2018 +0300
description:
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.

diffstat:

 src/http/modules/ngx_http_geo_module.c |  27 ++++++++++++++++-----------
 src/stream/ngx_stream_geo_module.c     |  27 ++++++++++++++++-----------
 2 files changed, 32 insertions(+), 22 deletions(-)

diffs (170 lines):

diff -r 0237af43d409 -r c69c13f10502 src/http/modules/ngx_http_geo_module.c
--- a/src/http/modules/ngx_http_geo_module.c	Wed Feb 21 15:50:35 2018 +0300
+++ b/src/http/modules/ngx_http_geo_module.c	Wed Feb 21 15:50:42 2018 +0300
@@ -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;
 }
 
 
diff -r 0237af43d409 -r c69c13f10502 src/stream/ngx_stream_geo_module.c
--- a/src/stream/ngx_stream_geo_module.c	Wed Feb 21 15:50:35 2018 +0300
+++ b/src/stream/ngx_stream_geo_module.c	Wed Feb 21 15:50:42 2018 +0300
@@ -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;
 }