changeset 7726:559d19037984

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.
author Maxim Dounin <mdounin@mdounin.ru>
date Thu, 08 Oct 2020 17:44:34 +0300
parents d63c5373b5ba
children f1e6f65ddfeb
files src/http/modules/ngx_http_limit_req_module.c
diffstat 1 files changed, 27 insertions(+), 15 deletions(-) [+]
line wrap: on
line diff
--- a/src/http/modules/ngx_http_limit_req_module.c
+++ b/src/http/modules/ngx_http_limit_req_module.c
@@ -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;