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	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 @@
     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 @@
         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 @@
                         &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 @@
 
 
 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;