Hello!
On Wed, Sep 27, 2023 at 01:13:44AM +0800, 上勾拳 wrote:
> Dear Nginx Developers,
>
> I hope this email finds you well. I am reaching out to the mailing list for
> the first time to report and discuss an issue I encountered while working
> on supporting PCRE2 in OpenResty. If I have made any errors in my reporting
> or discussion, please do not hesitate to provide feedback. Your guidance is
> greatly appreciated.
>
> During my recent work, I used the sanitizer to inspect potential issues,
> and I identified a small memory leak in the PCRE2 code section of Nginx.
> While this issue does not seem to be critical, it could potentially disrupt
> memory checking tools. To help you reproduce the problem, I have included a
> minimal configuration below. Please note that this issue occurs when Nginx
> is configured to use PCRE2, and the version is 1.22.1 or higher.
>
> *Minimal Configuration for Reproduction:*
> worker_processes 1;
> daemon off;
> master_process off;
> error_log
> /home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/servroot/logs/error.log
> debug;
> pid
> /home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/servroot/logs/nginx.pid;
>
> http {
> access_log
> /home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/servroot/logs/access.log;
> #access_log off;
> default_type text/plain;
> keepalive_timeout 68000ms;
> server {
> listen 1984;
> #placeholder
> server_name 'localhost';
>
> client_max_body_size 30M;
> #client_body_buffer_size 4k;
>
> # Begin preamble config...
>
> # End preamble config...
>
> # Begin test case config...
>
> location ~ '^/[a-d]$' {
> return 200;
> }
> }
> }
> events {
> accept_mutex off;
>
> worker_connections 64;
> }
>
> *nginx -V :*
> nginx version: nginx/1.25.1 (no pool)
> built by gcc 11.4.1 20230605 (Red Hat 11.4.1-2) (GCC)
> built with OpenSSL 1.1.1u 30 May 2023
> TLS SNI support enabled
> configure arguments:
> --prefix=/home/zhenzhongw/code/pcre_pr/lua-nginx-module/work/nginx
> --with-threads --with-pcre-jit --with-ipv6
> --with-cc-opt='-fno-omit-frame-pointer -fsanitize=address
> -DNGX_LUA_USE_ASSERT -I/opt/pcre2/include -I/opt/ssl/include'
> --with-http_v2_module --with-http_v3_module --with-http_realip_module
> --with-http_ssl_module
> --add-module=/home/zhenzhongw/code/pcre_pr/ndk-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/set-misc-nginx-module
> --with-ld-opt='-fsanitize=address -L/opt/pcre2/lib -L/opt/ssl/lib
> -Wl,-rpath,/opt/pcre2/lib:/opt/drizzle/lib:/opt/ssl/lib'
> --without-mail_pop3_module --without-mail_imap_module
> --with-http_image_filter_module --without-mail_smtp_module --with-stream
> --with-stream_ssl_module --without-http_upstream_ip_hash_module
> --without-http_memcached_module --without-http_auth_basic_module
> --without-http_userid_module --with-http_auth_request_module
> --add-module=/home/zhenzhongw/code/pcre_pr/echo-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/memc-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/srcache-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/lua-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/lua-upstream-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/headers-more-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/drizzle-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/rds-json-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/coolkit-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/redis2-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/stream-lua-nginx-module
> --add-module=/home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/data/fake-module
> --add-module=/home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/data/fake-shm-module
> --add-module=/home/zhenzhongw/code/pcre_pr/lua-nginx-module/t/data/fake-delayed-load-module
> --with-http_gunzip_module --with-http_dav_module --with-select_module
> --with-poll_module --with-debug --with-poll_module --with-cc=gcc
>
> *The sanitizer tool reported the following error message: *
> =================================================================
> ==555798==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 72 byte(s) in 1 object(s) allocated from:
> #0 0x7f502f6b4a07 in __interceptor_malloc (/lib64/libasan.so.6+0xb4a07)
> #1 0x4a1737 in ngx_alloc src/os/unix/ngx_alloc.c:22
> #2 0x525796 in ngx_regex_malloc src/core/ngx_regex.c:509
> #3 0x7f502f3e745e in _pcre2_memctl_malloc_8
> (/opt/pcre2/lib/libpcre2-8.so.0+0x1145e)
> #4 0x5771ad in ngx_http_regex_compile src/http/ngx_http_variables.c:2555
> #5 0x536088 in ngx_http_core_regex_location
> src/http/ngx_http_core_module.c:3263
> #6 0x537f94 in ngx_http_core_location
> src/http/ngx_http_core_module.c:3115
> #7 0x46ba0a in ngx_conf_handler src/core/ngx_conf_file.c:463
> #8 0x46ba0a in ngx_conf_parse src/core/ngx_conf_file.c:319
> #9 0x5391ec in ngx_http_core_server src/http/ngx_http_core_module.c:2991
> #10 0x46ba0a in ngx_conf_handler src/core/ngx_conf_file.c:463
> #11 0x46ba0a in ngx_conf_parse src/core/ngx_conf_file.c:319
> #12 0x528e4c in ngx_http_block src/http/ngx_http.c:239
> #13 0x46ba0a in ngx_conf_handler src/core/ngx_conf_file.c:463
> #14 0x46ba0a in ngx_conf_parse src/core/ngx_conf_file.c:319
> #15 0x463f74 in ngx_init_cycle src/core/ngx_cycle.c:284
> #12 0x528e4c in ngx_http_block src/http/ngx_http.c:239
> #13 0x46ba0a in ngx_conf_handler src/core/ngx_conf_file.c:463
> #14 0x46ba0a in ngx_conf_parse src/core/ngx_conf_file.c:319
> #15 0x463f74 in ngx_init_cycle src/core/ngx_cycle.c:284
> #16 0x4300c7 in main src/core/nginx.c:295
> #17 0x7ff31a43feaf in __libc_start_call_main (/lib64/libc.so.6+0x3feaf)
>
> SUMMARY: AddressSanitizer: 72 byte(s) leaked in 1 allocation(s).
>
> *I have created a patch to address this memory leak issue, which I am
> sharing below:*
> diff --git a/src/core/ngx_regex.c b/src/core/ngx_regex.c
> index 91381f499..71f583789 100644
> --- a/src/core/ngx_regex.c
> +++ b/src/core/ngx_regex.c
> @@ -600,6 +600,8 @@ ngx_regex_cleanup(void *data)
> * the new cycle, these will be re-allocated.
> */
>
> + ngx_regex_malloc_init(NULL);
> +
> if (ngx_regex_compile_context) {
> pcre2_compile_context_free(ngx_regex_compile_context);
> ngx_regex_compile_context = NULL;
> @@ -611,6 +613,8 @@ ngx_regex_cleanup(void *data)
> ngx_regex_match_data_size = 0;
> }
>
> + ngx_regex_malloc_done();
> +
> #endif
> }
>
> @@ -706,7 +710,13 @@ ngx_regex_module_init(ngx_cycle_t *cycle)
> ngx_regex_malloc_done();
>
> ngx_regex_studies = NULL;
> +
> #if (NGX_PCRE2)
> + if (ngx_regex_compile_context) {
> + ngx_regex_malloc_init(NULL);
> + pcre2_compile_context_free(ngx_regex_compile_context);
> + ngx_regex_malloc_done();
> + }
> ngx_regex_compile_context = NULL;
> #endif
>
> I kindly request your assistance in reviewing this matter and considering
> the patch for inclusion in Nginx. If you have any questions or need further
> information, please feel free to reach out to me. Your expertise and
> feedback are highly valuable in resolving this issue.
Thank you for the report.
Indeed, this looks like a small leak which manifests itself during
reconfiguration when nginx is compiled with PCRE2.
The patch looks correct to me, though I think it would be better
to don't do anything with ngx_regex_compile_context in
ngx_regex_module_init(). Please take a look if the following
patch looks good to you:
# HG changeset patch
# User Maxim Dounin <mdou...@mdounin.ru>
# Date 1696950530 -10800
# Tue Oct 10 18:08:50 2023 +0300
# Node ID 0ceb96f57592b77618fba4200797df977241ec9b
# Parent cdda286c0f1b4b10f30d4eb6a63fefb9b8708ecc
Core: fixed memory leak on configuration reload with PCRE2.
In ngx_regex_cleanup() allocator wasn't configured when calling
pcre2_compile_context_free() and pcre2_match_data_free(), resulting
in no ngx_free() call and leaked memory. Fix is ensure that allocator
is configured for global allocations, so that ngx_free() is actually
called to free memory.
Additionally, ngx_regex_compile_context was cleared in
ngx_regex_module_init(). It should be either not cleared, so it will
be freed by ngx_regex_cleanup(), or properly freed. Fix is to
not clear it, so ngx_regex_cleanup() will be able to free it.
Reported by ZhenZhong Wu,
https://mailman.nginx.org/pipermail/nginx-devel/2023-September/3Z5FIKUDRN2WBSL3JWTZJ7SXDA6YIWPB.html
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
@@ -600,6 +600,8 @@ ngx_regex_cleanup(void *data)
* the new cycle, these will be re-allocated.
*/
+ ngx_regex_malloc_init(NULL);
+
if (ngx_regex_compile_context) {
pcre2_compile_context_free(ngx_regex_compile_context);
ngx_regex_compile_context = NULL;
@@ -611,6 +613,8 @@ ngx_regex_cleanup(void *data)
ngx_regex_match_data_size = 0;
}
+ ngx_regex_malloc_done();
+
#endif
}
@@ -706,9 +710,6 @@ ngx_regex_module_init(ngx_cycle_t *cycle
ngx_regex_malloc_done();
ngx_regex_studies = NULL;
-#if (NGX_PCRE2)
- ngx_regex_compile_context = NULL;
-#endif
return NGX_OK;
}
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
