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

Fix for apr_global_mutex_create() crashes with mod_security 2.9.2 #1957

Closed
wants to merge 2 commits into from

Conversation

blappm
Copy link

@blappm blappm commented Nov 15, 2018

  • Call modsecurity_init() for the first invocation too.
  • Provide names/files for apr_global_mutex_create() to prevent segfaults in some cases:

[Thu Nov 8 10:58:40 2018] pid 16679 mod_backtrace main() is at 7f949bb6bd30
/usr/lib64/apache2/mod_backtrace.so(+0xddd)[0x7f949bc77ddd]
/usr/sbin/httpd2-prefork(ap_run_fatal_exception+0x4a)[0x7f949bb8b0ba]
/usr/sbin/httpd2-prefork(+0x4f446)[0x7f949bb8b446]
/lib64/libpthread.so.0(+0xf850)[0x7f949a46c850]
/usr/lib64/libapr-1.so.0(apr_global_mutex_lock+0x11)[0x7f949aad6a31]
/usr/lib64/apache2/mod_security2.so(+0x7155e)[0x7f949365655e]
/usr/lib64/apache2/mod_security2.so(+0x72213)[0x7f9493657213]
/usr/lib64/apache2/mod_security2.so(+0x75981)[0x7f949365a981]
/usr/lib64/apache2/mod_security2.so(+0x77ff0)[0x7f949365cff0]
/usr/lib64/apache2/mod_security2.so(+0x7a566)[0x7f949365f566]
/usr/lib64/apache2/mod_security2.so(+0x7bbb7)[0x7f9493660bb7]
/usr/lib64/apache2/mod_security2.so(+0x5be57)[0x7f9493640e57]
/usr/lib64/apache2/mod_security2.so(+0x5b8df)[0x7f94936408df]
/usr/sbin/httpd2-prefork(ap_run_post_read_request+0x4a)[0x7f949bb72cfa]
/usr/sbin/httpd2-prefork(ap_read_request+0xd55)[0x7f949bb75f35]
/usr/sbin/httpd2-prefork(+0x52250)[0x7f949bb8e250]
/usr/sbin/httpd2-prefork(ap_run_process_connection+0x83)[0x7f949bb89d33]
/usr/sbin/httpd2-prefork(+0x5a8de)[0x7f949bb968de]
/usr/sbin/httpd2-prefork(+0x5abca)[0x7f949bb96bca]
/usr/sbin/httpd2-prefork(ap_mpm_run+0x425)[0x7f949bb97055]

Apache 2.2.

Call modsecurity_init() for the first invocation too.
@blappm blappm changed the title Fix for apr_global_mutex_create() crashes Fix for apr_global_mutex_create() crashes with mod_security 2.9.2 Nov 15, 2018
@victorhora victorhora added enhancement 2.x Related to ModSecurity version 2.x labels Nov 16, 2018
@victorhora victorhora added this to the v2.9.3 milestone Nov 16, 2018
Avoid a unused variable warning.
@zimmerle
Copy link
Contributor

Hi @blappm,

Can you give more details on the crash and on your env?

@zimmerle zimmerle modified the milestones: v2.9.3, v2.9.4 Nov 26, 2018
@blappm
Copy link
Author

blappm commented Nov 26, 2018

Yes, of course.

The crashes happend on Suse Linux 12, SLES12, SP3 and SP4, but on different Apache versions (2.12 and 2.34). We've seen it on apache installed on ~10 systems, while 20 systems were running fine, maybe because there was less traffic. The special thing here is that apache-prefork from Suse is compiled with libpthread.so, even if it is not running threaded. This may result in the wrong locking mechanism beeing used, since flock/fcntl are not used because the filename in apr_global_mutex_create() is left empty.

There are other people reporting the similar crashes:

#564

From the HA-Proxy Archive I've also found the comment below, but I think the patch
is wrong for apache prefork, it only works for the threaded apache-worker:

https://www.mail-archive.com/haproxy@formilux.org/msg25681.html

`Modsecurity bugs:

  • When the audit_log is used with the directive "SecAuditLogType Serial", in
    some systems, the APR mutex initialisation silently fails, this causes a
    segmentation fault. For my own usage, I have a patched version of modsec where
    I use another mutex than "APR_LOCK_DEFAULT" like "APR_LOCK_PROC_PTHREAD"

    • rc = apr_global_mutex_create(&msce->auditlog_lock, NULL, APR_LOCK_DEFAULT, mp);
    • rc = apr_global_mutex_create(&msce->auditlog_lock, NULL, APR_LOCK_PROC_PTHREAD, mp);

`

}
modsecurity_init(modsecurity, mp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum.. I've got the change to use the tmp-file-names, but what about this change?

If i recall correctly this will be executed twice: while checking the configuration fro syntax error and while effective loading it. We may want to init ModSec only at the second one. Right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure about that. What about moving it then just after "first_time = 1;" ? Then it will really only be called once at the beginning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, we may not need to change it at all. I've created this branch v2/dev/pull_1957. It contains all the modifications except this one.

@zimmerle
Copy link
Contributor

Hi @blappm,

Thank you for the detailed explanation. I've made one more question along the code, about modsecurity_init(modsecurity, mp) being moved outside an else.

zimmerle pushed a commit that referenced this pull request Dec 10, 2018
@zimmerle
Copy link
Contributor

merged ;)

@zimmerle zimmerle closed this Dec 11, 2018
@emphazer
Copy link

finally! thx a lot for this one. we noticed that issue several times the past years.

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

Successfully merging this pull request may close these issues.

None yet

4 participants