[nginx] Limit req: unlocking of nodes on complex value errors.

[Maxim Dounin]

details:   https://hg.nginx.org/nginx/rev/559d19037984
branches:  
changeset: 7726:559d19037984
user:      Maxim Dounin <mdounin at mdounin.ru>
date:      Thu Oct 08 17:44:34 2020 +0300
description:
Limit req: unlocking of nodes on complex value errors.

Previously, if there were multiple limits configured, errors in
ngx_http_complex_value() during processing of a non-first limit
resulted in reference count leak in shared memory nodes of already
processed limits.  Fix is to explicity unlock relevant nodes, much
like we do when rejecting requests.

diffstat:

 src/http/modules/ngx_http_limit_req_module.c |  42 ++++++++++++++++++----------
 1 files changed, 27 insertions(+), 15 deletions(-)

diffs (73 lines):

diff -r d63c5373b5ba -r 559d19037984 src/http/modules/ngx_http_limit_req_module.c
--- a/src/http/modules/ngx_http_limit_req_module.c	Sat Oct 03 21:04:57 2020 +0300
+++ b/src/http/modules/ngx_http_limit_req_module.c	Thu Oct 08 17:44:34 2020 +0300
@@ -69,6 +69,8 @@ static ngx_int_t ngx_http_limit_req_look
     ngx_uint_t hash, ngx_str_t *key, ngx_uint_t *ep, ngx_uint_t account);
 static ngx_msec_t ngx_http_limit_req_account(ngx_http_limit_req_limit_t *limits,
     ngx_uint_t n, ngx_uint_t *ep, ngx_http_limit_req_limit_t **limit);
+static void ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t *limits,
+    ngx_uint_t n);
 static void ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx,
     ngx_uint_t n);
 
@@ -223,6 +225,7 @@ ngx_http_limit_req_handler(ngx_http_requ
         ctx = limit->shm_zone->data;
 
         if (ngx_http_complex_value(r, &ctx->key, &key) != NGX_OK) {
+            ngx_http_limit_req_unlock(limits, n);
             return NGX_HTTP_INTERNAL_SERVER_ERROR;
         }
 
@@ -270,21 +273,7 @@ ngx_http_limit_req_handler(ngx_http_requ
                         &limit->shm_zone->shm.name);
         }
 
-        while (n--) {
-            ctx = limits[n].shm_zone->data;
-
-            if (ctx->node == NULL) {
-                continue;
-            }
-
-            ngx_shmtx_lock(&ctx->shpool->mutex);
-
-            ctx->node->count--;
-
-            ngx_shmtx_unlock(&ctx->shpool->mutex);
-
-            ctx->node = NULL;
-        }
+        ngx_http_limit_req_unlock(limits, n);
 
         if (lrcf->dry_run) {
             r->main->limit_req_status = NGX_HTTP_LIMIT_REQ_REJECTED_DRY_RUN;
@@ -613,6 +602,29 @@ ngx_http_limit_req_account(ngx_http_limi
 
 
 static void
+ngx_http_limit_req_unlock(ngx_http_limit_req_limit_t *limits, ngx_uint_t n)
+{
+    ngx_http_limit_req_ctx_t  *ctx;
+
+    while (n--) {
+        ctx = limits[n].shm_zone->data;
+
+        if (ctx->node == NULL) {
+            continue;
+        }
+
+        ngx_shmtx_lock(&ctx->shpool->mutex);
+
+        ctx->node->count--;
+
+        ngx_shmtx_unlock(&ctx->shpool->mutex);
+
+        ctx->node = NULL;
+    }
+}
+
+
+static void
 ngx_http_limit_req_expire(ngx_http_limit_req_ctx_t *ctx, ngx_uint_t n)
 {
     ngx_int_t                   excess;