Extend PCRE malloc/free wrappers to use nginx pools

Author: defanator

src/ngx_http_modsecurity_body_filter.c

@@ -146,6 +146,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
     if (buffer_fully_loadead == 1)
     {
         int ret;
+        ngx_pool_t *old_pool;
 
         for (chain = in; chain != NULL; chain = chain->next)
         {
@@ -159,9 +160,9 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
             }
         }
 
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
         msc_process_response_body(ctx->modsec_transaction);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
 
 /* XXX: I don't get how body from modsec being transferred to nginx's buffer.  If so - after adjusting of nginx's
    XXX: body we can proceed to adjust body size (content-length).  see xslt_body_filter() for example */

src/ngx_http_modsecurity_common.h

@@ -90,8 +90,8 @@ extern ngx_module_t ngx_http_modsecurity_module;
 int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r);
 ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r);
 char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p);
-void ngx_http_modsecurity_pcre_malloc_init(void);
-void ngx_http_modsecurity_pcre_malloc_done(void);
+ngx_pool_t *ngx_http_modsecurity_pcre_malloc_init(ngx_pool_t *pool);
+void ngx_http_modsecurity_pcre_malloc_done(ngx_pool_t *old_pool);
 
 /* ngx_http_modsecurity_body_filter.c */
 ngx_int_t ngx_http_modsecurity_body_filter_init(void);

src/ngx_http_modsecurity_header_filter.c

@@ -415,6 +415,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
     int ret = 0;
     ngx_uint_t status;
     char *http_response_ver;
+    ngx_pool_t *old_pool;
 
 
 /* XXX: if NOT_MODIFIED, do we need to process it at all?  see xslt_header_filter() */
@@ -518,9 +519,9 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
     }
 #endif
 
-    ngx_http_modsecurity_pcre_malloc_init();
+    old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
     msc_process_response_headers(ctx->modsec_transaction, status, http_response_ver);
-    ngx_http_modsecurity_pcre_malloc_done();
+    ngx_http_modsecurity_pcre_malloc_done(old_pool);
     ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
     if (ret > 0) {
         return ret;

src/ngx_http_modsecurity_log.c

@@ -37,6 +37,7 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
 {
     ngx_http_modsecurity_ctx_t *ctx = NULL;
     ngx_http_modsecurity_loc_conf_t *cf;
+    ngx_pool_t *old_pool;
 
     dd("catching a new _log_ phase handler");
 
@@ -65,9 +66,9 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
     }
 
     dd("calling msc_process_logging for %p", ctx);
-    ngx_http_modsecurity_pcre_malloc_init();
+    old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
     msc_process_logging(ctx->modsec_transaction);
-    ngx_http_modsecurity_pcre_malloc_done();
+    ngx_http_modsecurity_pcre_malloc_done(old_pool);
 
     return NGX_OK;
 }

src/ngx_http_modsecurity_module.c

@@ -19,6 +19,7 @@
 #include "ddebug.h"
 
 #include "ngx_http_modsecurity_common.h"
+#include "stdio.h"
 
 static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf);
 static void *ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf);
@@ -28,32 +29,74 @@ static void ngx_http_modsecurity_main_config_cleanup(void *data);
 static void ngx_http_modsecurity_config_cleanup(void *data);
 
 /*
- * pcre malloc/free hack magic
+ * PCRE malloc/free workaround, based on
+ * https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_pcrefix.c
  */
+
 static void *(*old_pcre_malloc)(size_t);
 static void (*old_pcre_free)(void *ptr);
+static ngx_pool_t *ngx_http_modsec_pcre_pool = NULL;
 
-void
-ngx_http_modsecurity_pcre_malloc_init(void)
+static void *
+ngx_http_modsec_pcre_malloc(size_t size)
 {
-    old_pcre_malloc = pcre_malloc;
-    old_pcre_free = pcre_free;
+    if (ngx_http_modsec_pcre_pool) {
+        return ngx_palloc(ngx_http_modsec_pcre_pool, size);
+    }
 
-    pcre_malloc = malloc;
-    pcre_free = free;
+    fprintf(stderr, "error: modsec pcre malloc failed due to empty pcre pool");
+
+    return NULL;
 }
 
-void
-ngx_http_modsecurity_pcre_malloc_done(void)
+static void
+ngx_http_modsec_pcre_free(void *ptr)
 {
-    if (old_pcre_malloc == NULL)
+    if (ngx_http_modsec_pcre_pool) {
+        ngx_pfree(ngx_http_modsec_pcre_pool, ptr);
         return;
+    }
+
+#if 0
+    /* this may happen when called from cleanup handlers */
+    fprintf(stderr, "error: modsec pcre free failed due to empty pcre pool");
+#endif
+
+    return;
+}
+
+ngx_pool_t *
+ngx_http_modsecurity_pcre_malloc_init(ngx_pool_t *pool)
+{
+    ngx_pool_t  *old_pool;
+
+    if (pcre_malloc != ngx_http_modsec_pcre_malloc) {
+        ngx_http_modsec_pcre_pool = pool;
+
+        old_pcre_malloc = pcre_malloc;
+        old_pcre_free = pcre_free;
 
-    pcre_malloc = old_pcre_malloc;
-    pcre_free = old_pcre_free;
+        pcre_malloc = ngx_http_modsec_pcre_malloc;
+        pcre_free = ngx_http_modsec_pcre_free;
 
-    old_pcre_malloc = NULL;
-    old_pcre_free = NULL;
+        return NULL;
+    }
+
+    old_pool = ngx_http_modsec_pcre_pool;
+    ngx_http_modsec_pcre_pool = pool;
+
+    return old_pool;
+}
+
+void
+ngx_http_modsecurity_pcre_malloc_done(ngx_pool_t *old_pool)
+{
+    ngx_http_modsec_pcre_pool = old_pool;
+
+    if (old_pool == NULL) {
+        pcre_malloc = old_pcre_malloc;
+        pcre_free = old_pcre_free;
+    }
 }
 
 /*
@@ -494,6 +537,7 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
 {
     ngx_http_modsecurity_loc_conf_t *p = NULL;
     ngx_http_modsecurity_loc_conf_t *c = NULL;
+    ngx_pool_t *old_pool;
 
     p = parent;
     c = child;
@@ -529,9 +573,9 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
         if (rules_remote_key == (char *)-1) {
             return NGX_CONF_ERROR;
         }
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool);
         res = msc_rules_add_remote(c->rules_set, rules_remote_key, rules_remote_server, &error);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
         dd("Loading rules from: '%s'", rules_remote_server);
         if (res < 0) {
             dd("Failed to load the rules from: '%s'  - reason: '%s'", rules_remote_server, error);
@@ -547,9 +591,9 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
         if (rules_set == (char *)-1) {
             return NGX_CONF_ERROR;
         }
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool);
         res = msc_rules_add_file(c->rules_set, rules_set, &error);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
         dd("Loading rules from: '%s'", rules_set);
         if (res < 0) {
             dd("Failed to load the rules from: '%s' - reason: '%s'", rules_set, error);
@@ -565,9 +609,9 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
         if (rules == (char *)-1) {
             return NGX_CONF_ERROR;
         }
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool);
         res = msc_rules_add(c->rules_set, rules, &error);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
         dd("Loading rules: '%s'", rules);
         if (res < 0) {
             dd("Failed to load the rules: '%s' - reason: '%s'", rules, error);
@@ -584,20 +628,31 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
 static void
 ngx_http_modsecurity_main_config_cleanup(void *data)
 {
+    ngx_pool_t *old_pool;
     ngx_http_modsecurity_main_conf_t *cf = data;
+
+    dd("deleting a main conf %p", data);
+
+    old_pool = ngx_http_modsecurity_pcre_malloc_init(NULL);
     msc_cleanup(cf->modsec);
+    ngx_http_modsecurity_pcre_malloc_done(old_pool);
+
     cf->modsec = NULL;
 }
 
 
 static void
 ngx_http_modsecurity_config_cleanup(void *data)
 {
+    ngx_pool_t *old_pool;
     ngx_http_modsecurity_loc_conf_t *t = (ngx_http_modsecurity_loc_conf_t *) data;
+
     dd("deleting a loc conf -- RuleSet is: \"%p\"", t->rules_set);
-    ngx_http_modsecurity_pcre_malloc_init();
+
+    old_pool = ngx_http_modsecurity_pcre_malloc_init(NULL);
     msc_rules_cleanup(t->rules_set);
-    ngx_http_modsecurity_pcre_malloc_done();
+    ngx_http_modsecurity_pcre_malloc_done(old_pool);
+
     t->rules_set = NULL;
 }
 

src/ngx_http_modsecurity_pre_access.c

@@ -45,6 +45,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
 #if 1
     ngx_http_modsecurity_ctx_t *ctx = NULL;
     ngx_http_modsecurity_loc_conf_t *cf;
+    ngx_pool_t *old_pool;
 
     dd("catching a new _preaccess_ phase handler");
 
@@ -194,9 +195,9 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
 
 /* XXX: once more -- is body can be modified ?  content-length need to be adjusted ? */
 
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
         msc_process_request_body(ctx->modsec_transaction);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
 
         ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
         if (ret > 0) {

src/ngx_http_modsecurity_rewrite.c

@@ -25,6 +25,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
 {
     ngx_http_modsecurity_ctx_t *ctx = NULL;
     ngx_http_modsecurity_loc_conf_t *cf;
+    ngx_pool_t *old_pool;
 
     cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module);
     if (cf == NULL || cf->enable != 1) {
@@ -82,11 +83,11 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
             return NGX_HTTP_INTERNAL_SERVER_ERROR;
         }
         const char *server_addr = inet_ntoa(((struct sockaddr_in *) connection->sockaddr)->sin_addr);
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
         ret = msc_process_connection(ctx->modsec_transaction,
             client_addr, client_port,
             server_addr, server_port);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
         if (ret != 1){
             dd("Was not able to extract connection information.");
         }
@@ -129,9 +130,9 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
         if (n_uri == (char*)-1 || n_method == (char*)-1) {
             return NGX_HTTP_INTERNAL_SERVER_ERROR;
         }
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
         msc_process_uri(ctx->modsec_transaction, n_uri, n_method, http_version);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
 
         dd("Processing intervention with the transaction information filled in (uri, method and version)");
         ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
@@ -178,9 +179,9 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
          * to process this information.
          */
 
-        ngx_http_modsecurity_pcre_malloc_init();
+        old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
         msc_process_request_headers(ctx->modsec_transaction);
-        ngx_http_modsecurity_pcre_malloc_done();
+        ngx_http_modsecurity_pcre_malloc_done(old_pool);
         dd("Processing intervention with the request headers information filled in");
         ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
         if (ret > 0) {