Skip to content

Memory leaks with nginx reload (without restart) #2848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martinhsv opened this issue Dec 21, 2022 · 21 comments
Open

Memory leaks with nginx reload (without restart) #2848

martinhsv opened this issue Dec 21, 2022 · 21 comments
Labels
3.x Related to ModSecurity version 3.x Platform - Nginx

Comments

@martinhsv
Copy link
Contributor

When, instead of restarting nginx, one performs a reload of the configuration, memory may leak.

The memory leaks are not large per reload, but if doing so frequently, the free memory reduction will become noticeable. A near-term mitigation is to at least periodically do a true restart.

This issue is being created in lieu of #2502 and others.

@Volatus
Copy link

Volatus commented Jan 26, 2023

@martinhsv Would it be possible to do a small patch release soon that would include the commit e9a7ba4 ? According to kubernetes/ingress-nginx#8166 (comment) it did alleviate the leaks that were significant in size and would be very helpful for the users for the time being as other leaks related to reloading get ironed out. Thanks.

@IanRobertson-wpe
Copy link

Hi @martinhsv, I believe my organization is encountering this issue as well. What is needed in order to address this? Is the root cause already identified?

@martinhsv
Copy link
Contributor Author

Hello @Volatus ,

The ModSecurity project has not had a practice of doing patch releases for single issues that are separate from accumulated changes in v3/master. When the next 3.0.x release appears, it will almost certainly be from v3/master as that branch appears at that time.

The next release of 3.0.x is tentatively planned for the moderately soon period (by which I mean within 8-10 weeks) -- if that is within your definition of 'soon'. If that is problematically long for you, you could always build ModSecurity yourself from a point in time that includes the commit that you have referenced.

I do, understand, of course, that some installations may have a policy of only allowing use of official releases. If that is the case for you, and the aforementioned timeframe is problematic, I could take that into consideration. But that would have to be weighed against some other factors.

Hi @IanRobertson-wpe ,

Perhaps my response to Volatus answers most of what you are asking about as well.

One other thing I'll mention is that there is one (smaller) unresolved leak present with common ModSecurity usage. I have a fix for this for which I'll likely create the PR within a few days.

@Volatus
Copy link

Volatus commented Jan 31, 2023

Hello @Volatus ,

The ModSecurity project has not had a practice of doing patch releases for single issues that are separate from accumulated changes in v3/master. When the next 3.0.x release appears, it will almost certainly be from v3/master as that branch appears at that time.

The next release of 3.0.x is tentatively planned for the moderately soon period (by which I mean within 8-10 weeks) -- if that is within your definition of 'soon'. If that is problematically long for you, you could always build ModSecurity yourself from a point in time that includes the commit that you have referenced.

I do, understand, of course, that some installations may have a policy of only allowing use of official releases. If that is the case for you, and the aforementioned timeframe is problematic, I could take that into consideration. But that would have to be weighed against some other factors.

Hi @IanRobertson-wpe ,

Perhaps my response to Volatus answers most of what you are asking about as well.

One other thing I'll mention is that there is one (smaller) unresolved leak present with common ModSecurity usage. I have a fix for this for which I'll likely create the PR within a few days.

Thanks for the response. I think we may have a temporary fix that allows us to point to the specific commit that includes the memory leak fix. 8-10 weeks is not bad at all assuming our fix works for the time being.

@martinhsv
Copy link
Contributor Author

martinhsv commented Feb 21, 2023

There is a pull request available ( #2876 ) that resolves the most prominent remaining memory leak issue when doing an nginx reload (rather than restart).

The leak in this case is proportional to the number of '.conf' ModSecurity files being read in by the bison parser.

Users for whom this is a significant nuisance in the immediate period are invited to try out the PR during the period before it is merged.

@martinhsv
Copy link
Contributor Author

Known memory leaks of general application that occur when executing nginx reload have been resolved as of v3.0.9.

I will, however, leave this item open for at least a few weeks to allow for any additional reports.

It is probable that any further memory leaks associated with reload are related to less-commonly used features. Specific information about what sorts of Sec* constructs appear to trigger the effect would be helpful.

@S0obi
Copy link

S0obi commented May 13, 2023

Hello @martinhsv, here is a report from our side.

Configuration : nginx 1.23.4 with modsecurity 3.0.9 (Ubuntu 20.04.6), coreruleset-3.3.4 with few custom rules. 197 servers blocks in nginx configs with 191 "modsecurity on" directives.

Here are few tests I performed and data I collected :

  • Reloading nginx multiple times (systemctl reload nginx) with ModSecurity 3.0.9 will not consume more memory. With modsecurity < 3.0.9, multiple reload will increase the used amount of memory
  • I restarted nginx (systemctl restart nginx), after few second the used memory usage will stabilize around 2.53Go. After a first reload, the memory will peak and stabilize at 4.53 Go. Second and third reload after will not change the used memory (I can only a short burst during reload to 4.62Go and then get back to 4.53Go)

However, I am still observing a rampant memory leak over time. Here is a screenshot of the memory usage of our production server (similar to test server):

Screenshot 2023-05-13 at 17 59 16

This situation is not new to modsecurity 3.0.9 but was also noticeable in the previous versions.

@Zoey2936
Copy link

Known memory leaks of general application that occur when executing nginx reload have been resolved as of v3.0.9.

I still get one. Running nginx -s reload makes my server using all 2GB of Ram and 4GB of swap, also cpu goes up to 100% until the server freezes, killing and relaunching instead makes the server use 1,5GB ram and 3GB of swap with the cpu being at nearly 0%. I use CRS and the default modsec configuration.

@GNU-Plus-Windows-User
Copy link

I can also confirm the memory leak on reload still exists. personally, I've found the memory leak size is based on the number of modsecurity on; and modsecurity_rules_file /path/to/example.conf; directives and the use of @pmFromFile.

@martinhsv
Copy link
Contributor Author

Thanks to all for the ongoing reports

@GNU-Plus-Windows-User : thanks for the specific reference to @pmFromFile.

Interestingly, the suspect functionality seems to have already been noted quite some time ago: https://github.com/SpiderLabs/ModSecurity/blame/b84f32d6f2e2e024cd85d82c6707ce66327eb7d0/src/utils/acmp.cc#L32

... but it had not previously come to my attention.

@GNU-Plus-Windows-User
Copy link

@martinhsv Awesome, at least you are aware of it now.
I'm happy to test any PRs that may fix this issue.

@Zoey2936
Copy link

Will this problem be fixed in the next time?

@martinhsv
Copy link
Contributor Author

martinhsv commented Jun 26, 2023

Hi @GNU-Plus-Windows-User ,

Regarding your specific reference to @pmFromFile, I have not yet been able to identify a memory leak in that code. (The code comment in acmp.cc that I previously mentioned may be obsolete since at least one correction was made later in time than that comment).

One possibility is that there is a distinctive use case in your file(s) that was not replicated in the tests that I ran with valgrind. Are you able/willing to share yours? ( If there is anything sensitive in the content, you could send them to the address listed here: https://github.com/SpiderLabs/ModSecurity#security-issue , rather than including them here.)

Also: are you using local files for that operator? Or are you using the https: option?

@martinhsv
Copy link
Contributor Author

Hi @GNU-Plus-Windows-User ,

Regarding your observations about modsecurity on and modsecurity_rules_file ...

Could you tell me more about your environment? nginx version? ModSecurity version and Connector (ModSecurity-nginx) versions? Are you using PCRE1 or PCRE2?

Part of the reason I ask is that it's possible the leaks are actually related to code in the separate connector project (ModSecurity-nginx). One factor in that code is that there is some more complex memory pool management when PCRE1 is being used.

@S0obi , for the above reason: were you using PCRE1 or PCRE2 (the former is still the default)?

@S0obi
Copy link

S0obi commented Jun 26, 2023

Hey @martinhsv, modsec library has been built with ./configure --with-pcre2 so PCRE2.

@GNU-Plus-Windows-User
Copy link

@martinhsv I'm running the latest version of both modsec and the connector, my ModSecurity is compiled with pcre2 and pcre-jit for the nginx connector.
I'm using nginx 1.24.0 on ubuntu 22.04 from suvy's repo and I have about 9 server blocks with 9 modsecurity on and modsecurity_rules_file directives.
I should note that I'm observing the same/similar patterns with the memory leak as s0obi.

@S0obi
Copy link

S0obi commented Aug 2, 2023

@martinhsv any clue about what can be one or multiple causes of this memory leak ?

@martinhsv
Copy link
Contributor Author

martinhsv commented Sep 12, 2023

I have identified some leaks when using the --with-lmdb option. I will be addressing those shortly.

[Edit: the issue mentioned here was resolved in #2983 . Note that those leaks are not restricted to rule reload but can occur during each transaction.]

@S0obi
Copy link

S0obi commented Mar 11, 2025

As this is the most complete topic on Modsecurity memory leaks, I will document here some of the findings that may help to find new paths for improving the situation.

Summary

On reload operation, nginx modsecurity module cleanup functions will be called (ngx_http_modsecurity_cleanup_rules and ngx_http_modsecurity_cleanup_instance) which will free Modsecurity and RuleSet structures. Comparing the number of calls between reload and restart seems to be equivalent. Quick review of ngx_http_modsecurity_merge_conf should be done to check if it can have a role with memory leaks.

How does it work in detail ?

Starting with documentation

According to Nginx documentation :

In order for nginx to re-read the configuration file, a HUP signal should be sent to the master process. [...] If this succeeds, it starts new worker processes, and sends messages to old worker processes requesting them to shut down gracefully.

In the development guide, we have a bit more information about cycle :

Each time the nginx configuration is reloaded, a new cycle is created from the new nginx configuration; the old cycle is usually deleted after the new one is successfully created.

On cleanup mecanism :

Cleanup handlers can be registered in a pool. A cleanup handler is a callback with an argument which is called when pool is destroyed. A pool is usually tied to a specific nginx object (like an HTTP request) and is destroyed when the object reaches the end of its lifetime. Registering a pool cleanup is a convenient way to release resources, close file descriptors or make final adjustments to the shared data associated with the main object.

Understanding how nginx handles SIGHUP with code

A source code analysis of nginx will help us understand the documentation better.
The HUP signal is referenced by the NGX_RECONFIGURE_SIGNAL macro and captured with sigaddset() in ngx_master_process_cycle() function.

The important source code controlling nginx behavior on reload is in ngx_process_cycle.c :

ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "reconfiguring");


cycle = ngx_init_cycle(cycle);
if (cycle == NULL) {
    cycle = (ngx_cycle_t *) ngx_cycle;
    continue;
}


ngx_cycle = cycle;
ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
                                        ngx_core_module);
ngx_start_worker_processes(cycle, ccf->worker_processes,
                            NGX_PROCESS_JUST_RESPAWN);
ngx_start_cache_manager_processes(cycle, 1);


/* allow new processes to start */
ngx_msleep(100);


live = 1;
ngx_signal_worker_processes(cycle,
                            ngx_signal_value(NGX_SHUTDOWN_SIGNAL));

On the reload operation, ngx_destroy_pool() will call the cleanup handler attached to the pool.

Experimenting with Modsecurity-nginx module

To enable interesting debug logs, we can just uncomment a few lines from src/ddebug.h.

A typical reload operation will call these function (in this specific order) :

  1. ngx_http_modsecurity_create_main_conf (once)
  2. ngx_http_modsecurity_create_conf (multiple times)
  3. ngx_http_modsecurity_merge_conf (multiple times)
  4. ngx_http_modsecurity_cleanup_rules (multiple times)
  5. ngx_http_modsecurity_cleanup_instance (multiple times)

A comparison between reload and restart is showing the same number of cleanup operations.

Hypothesis: is ngx_http_modsecurity_merge_conf doing memory allocations that are not free ? When calling msc_rules_merge, what happens to the previous ruleset ?

Observation : after a fresh start, the memory allocated for nginx is normal, after a first reload, we can observe a bump in the allocated memory (+3Go on our test instance). Second and next reload will bump the allocated memory temporarily (that can cause an OOM) and come back to previous level plus a small amount (+10Mo on our test instance after the next reload). Finally, memory allocated will slowly increase over time.

@wohaiaini
Copy link

Is it possible that this is caused by memory fragmentation?

@airween
Copy link
Member

airween commented Mar 12, 2025

Is it possible that this is caused by memory fragmentation?

I don't think so. It's definitely there when you have so much free memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x Platform - Nginx
Projects
None yet
Development

No branches or pull requests

9 participants