[PATCH 1 of 4] Core: fixed ngx_pcre_studies cleanup

Maxim Dounin mdounin at mdounin.ru
Sun Dec 12 11:22:37 UTC 2021


# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1639235685 -10800
#      Sat Dec 11 18:14:45 2021 +0300
# Node ID 7b0522664bd790fda5d25df9087759953b30ac29
# Parent  a7a77549265ef46f1f0fdb3897f4beabf9e09c40
Core: fixed ngx_pcre_studies cleanup.

If a configuration parsing fails for some reason, ngx_regex_module_init()
is not called, and ngx_pcre_studies remained set despite the fact that
the pool it was allocated from is already freed.  This might result in
a segmentation fault during runtime regular expression compilation, such
as in SSI, for example, in the single process mode, or if a worker process
dies and respawn from a master process in such an inconsistent state.

Fix is to clear ngx_pcre_studies from the pool cleanup handler (which is
anyway used to free JIT-compiled patterns).

diff --git a/src/core/ngx_regex.c b/src/core/ngx_regex.c
--- a/src/core/ngx_regex.c
+++ b/src/core/ngx_regex.c
@@ -10,15 +10,14 @@
 
 
 typedef struct {
-    ngx_flag_t  pcre_jit;
+    ngx_flag_t   pcre_jit;
+    ngx_list_t  *studies;
 } ngx_regex_conf_t;
 
 
 static void * ngx_libc_cdecl ngx_regex_malloc(size_t size);
 static void ngx_libc_cdecl ngx_regex_free(void *p);
-#if (NGX_HAVE_PCRE_JIT)
-static void ngx_pcre_free_studies(void *data);
-#endif
+static void ngx_regex_cleanup(void *data);
 
 static ngx_int_t ngx_regex_module_init(ngx_cycle_t *cycle);
 
@@ -248,18 +247,17 @@ ngx_regex_free(void *p)
 }
 
 
+static void
+ngx_regex_cleanup(void *data)
+{
 #if (NGX_HAVE_PCRE_JIT)
-
-static void
-ngx_pcre_free_studies(void *data)
-{
-    ngx_list_t *studies = data;
+    ngx_regex_conf_t *rcf = data;
 
     ngx_uint_t        i;
     ngx_list_part_t  *part;
     ngx_regex_elt_t  *elts;
 
-    part = &studies->part;
+    part = &rcf->studies->part;
     elts = part->elts;
 
     for (i = 0; /* void */ ; i++) {
@@ -274,56 +272,50 @@ ngx_pcre_free_studies(void *data)
             i = 0;
         }
 
-        if (elts[i].regex->extra != NULL) {
-            pcre_free_study(elts[i].regex->extra);
-        }
-    }
-}
-
-#endif
-
-
-static ngx_int_t
-ngx_regex_module_init(ngx_cycle_t *cycle)
-{
-    int               opt;
-    const char       *errstr;
-    ngx_uint_t        i;
-    ngx_list_part_t  *part;
-    ngx_regex_elt_t  *elts;
-
-    opt = 0;
-
-#if (NGX_HAVE_PCRE_JIT)
-    {
-    ngx_regex_conf_t    *rcf;
-    ngx_pool_cleanup_t  *cln;
-
-    rcf = (ngx_regex_conf_t *) ngx_get_conf(cycle->conf_ctx, ngx_regex_module);
-
-    if (rcf->pcre_jit) {
-        opt = PCRE_STUDY_JIT_COMPILE;
-
         /*
          * The PCRE JIT compiler uses mmap for its executable codes, so we
          * have to explicitly call the pcre_free_study() function to free
          * this memory.
          */
 
-        cln = ngx_pool_cleanup_add(cycle->pool, 0);
-        if (cln == NULL) {
-            return NGX_ERROR;
+        if (elts[i].regex->extra != NULL) {
+            pcre_free_study(elts[i].regex->extra);
         }
+    }
+#endif
 
-        cln->handler = ngx_pcre_free_studies;
-        cln->data = ngx_pcre_studies;
-    }
+    /*
+     * On configuration parsing errors ngx_regex_module_init() will not
+     * be called.  Make sure ngx_pcre_studies is properly cleared anyway.
+     */
+
+    ngx_pcre_studies = NULL;
+}
+
+
+static ngx_int_t
+ngx_regex_module_init(ngx_cycle_t *cycle)
+{
+    int                opt;
+    const char        *errstr;
+    ngx_uint_t         i;
+    ngx_list_part_t   *part;
+    ngx_regex_elt_t   *elts;
+    ngx_regex_conf_t  *rcf;
+
+    opt = 0;
+
+    rcf = (ngx_regex_conf_t *) ngx_get_conf(cycle->conf_ctx, ngx_regex_module);
+
+#if (NGX_HAVE_PCRE_JIT)
+    if (rcf->pcre_jit) {
+        opt = PCRE_STUDY_JIT_COMPILE;
     }
 #endif
 
     ngx_regex_malloc_init(cycle->pool);
 
-    part = &ngx_pcre_studies->part;
+    part = &rcf->studies->part;
     elts = part->elts;
 
     for (i = 0; /* void */ ; i++) {
@@ -374,7 +366,8 @@ ngx_regex_module_init(ngx_cycle_t *cycle
 static void *
 ngx_regex_create_conf(ngx_cycle_t *cycle)
 {
-    ngx_regex_conf_t  *rcf;
+    ngx_regex_conf_t    *rcf;
+    ngx_pool_cleanup_t  *cln;
 
     rcf = ngx_pcalloc(cycle->pool, sizeof(ngx_regex_conf_t));
     if (rcf == NULL) {
@@ -383,11 +376,21 @@ ngx_regex_create_conf(ngx_cycle_t *cycle
 
     rcf->pcre_jit = NGX_CONF_UNSET;
 
-    ngx_pcre_studies = ngx_list_create(cycle->pool, 8, sizeof(ngx_regex_elt_t));
-    if (ngx_pcre_studies == NULL) {
+    cln = ngx_pool_cleanup_add(cycle->pool, 0);
+    if (cln == NULL) {
         return NULL;
     }
 
+    cln->handler = ngx_regex_cleanup;
+    cln->data = rcf;
+
+    rcf->studies = ngx_list_create(cycle->pool, 8, sizeof(ngx_regex_elt_t));
+    if (rcf->studies == NULL) {
+        return NULL;
+    }
+
+    ngx_pcre_studies = rcf->studies;
+
     return rcf;
 }
 



More information about the nginx-devel mailing list