Skip to content
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

Nginx mod_security leaks file descriptors #137

Closed
kirilkalchev opened this issue Aug 22, 2013 · 16 comments
Closed

Nginx mod_security leaks file descriptors #137

kirilkalchev opened this issue Aug 22, 2013 · 16 comments

Comments

@kirilkalchev
Copy link

Hi,

I have a problem with nginx and mod_security module. After reloading nginx configuration (kill -HUP ) all files opened by mod_security are opened once again without closing the old ones. That means at some point we hit the limit of open file descriptors, in my real life scenario I leak over 300 files on each reload.

Here are my sample configs just to illustrate the problem:

nginx.conf
user www-data www-data;
worker_processes 6;
worker_rlimit_nofile 200000;

error_log /var/log/nginx/error.log debug;

events {
worker_connections 16384;
multi_accept on;
use epoll;
}

http {
server {
listen 80;
location / {
ModSecurityEnabled on;
ModSecurityConfig modsecurity.conf;
return 555;
}
}
}

modsecurity.conf:
SecDebugLog /var/log/waf/events.log

In this situation after each configuration reload I am leaking open files:

www-data@dev03 ~ # lsof | grep nginx | wc -l; kill -HUP ps aux | grep 'nginx: master process' | grep -v grep | awk '{print $2}'; sleep 2; lsof | grep nginx | wc -l
361
368

I am using Ubuntu 12.04 LTS and nginx _openresty 1.4.2.1

(DEPLOY)www-data@dev03:~# nginx -V
nginx version: ngx_openresty/1.4.2.1
built by gcc 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)
TLS SNI support enabled

I will be happy to provide other information if necessary.

Regards,
Kiril

@brenosilva
Copy link
Contributor

We will take a look why nginx is not closing the files. I suppose it is also happening if you do "nginx reload" right ? @chaizhenhua any idea ?

@chaizhenhua
Copy link
Contributor

@brenosilva I will take a look at it.

@chaizhenhua
Copy link
Contributor

fixed by #139

@brenosilva
Copy link
Contributor

Hello @kirilkalchev
Could you please try download the latest trunk and test ?

Let us know your feedback.

Thanks

Breno

@kirilkalchev
Copy link
Author

I am gonna try it till the end of the week.

Regards,
Kiril

On Sep 4, 2013, at 3:15 PM, Breno Silva wrote:

Hello @kirilkalchev
Could you please try download the latest trunk and test ?

Let us know your feedback.

Thanks

Breno


Reply to this email directly or view it on GitHub.

@kirilkalchev
Copy link
Author

Sorry for the delay. It looks great.

Thank you,
Kiril

On Sep 4, 2013, at 5:02 PM, Kiril Kalchev wrote:

I am gonna try it till the end of the week.

Regards,
Kiril

On Sep 4, 2013, at 3:15 PM, Breno Silva wrote:

Hello @kirilkalchev
Could you please try download the latest trunk and test ?

Let us know your feedback.

Thanks

Breno


Reply to this email directly or view it on GitHub.

@zimmerle
Copy link
Contributor

Hi @brenosilva,

I have checked the issue using the same configurations that was presented by @kirilkalchev against the trunk branch and looks like it is working fine.

(/usr/local/nginx/sbin)# (echo -n " Number of FHs before 'kill -HUP': " ; lsof -n 2> /dev/null | grep nginx | wc -l) && (kill -HUP ps aux | grep 'nginx: master process' | grep -v grep | awk '{print $2}') && (sleep 2; echo -n "Number of FHs after the 'kill -HUP': " ; lsof -n 2>/dev/null | grep nginx | wc -l)
Number of FHs before 'kill -HUP': 271
Number of FHs after the 'kill -HUP': 271

There is any other test that you are considering in order to close this issue?

Thanks,
F.

@kirilkalchev
Copy link
Author

Hi,

When do you plan to make a release with this fix? I don't like the idea to use some unknown version in production.

Regards,
Kiril

@rcbarnett-zz
Copy link
Contributor

Should be in our next release - v2.7.6

@zimmerle
Copy link
Contributor

Opening the bug again as explained on: #579.

@prdevel006
Copy link

Entire memory and registering the hooks is done in master process and forked to worker process right?. Why we need to allocate memory in master process even though its not needed there.

@zimmerle
Copy link
Contributor

zimmerle commented Nov 9, 2015

To be fixed in libmodsecurity

@LinuxJedi
Copy link
Contributor

@zimmerle this is tagged with libmodsecurity but I can't seem to reproduce it with ModSecurity-nginx with SecDebugLog used. Is it still an issue?

[root@ngx1 ~]# lsof | grep nginx | wc -l; kill -HUP `ps aux | grep 'nginx: master process' | grep -v grep | awk '{print $2}'`; sleep 2; lsof | grep nginx | wc -l
881
881

@zimmerle
Copy link
Contributor

Hi @LinuxJedi,

This issue is not on the libmodsecurity implementation. It is an ModSecurity 2.x issue. The "libmodsecurity" tag here means that it won't be fixed in 2.x.

@LinuxJedi
Copy link
Contributor

many thanks for letting me know. Maybe this should be closed as "Won't fix in 2.x and fixed in libmodsecurity" then?

@zimmerle
Copy link
Contributor

zimmerle commented May 6, 2017

Won't fix in 2.x and fixed in libmodsecurity

Further information available here - https://github.com/SpiderLabs/ModSecurity-nginx

@zimmerle zimmerle closed this as completed May 6, 2017
@zimmerle zimmerle self-assigned this May 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants