Issue 890 - ModSecurity corrupts the global pool's cleanups linked list with threaded MPMs. #2049
Conversation
The code for testing regexes with embedded Apache variables (rule->re_precomp == 1) during request processing was utilizing the global engine pool for the storage of temporary values. This approach is not threadsafe, retains the temporary variables longer than they are usable, and causes corruption of the global pool's "cleanups" linked-lists when Apache is configured with a threaded MPM.
victorhora
added
Platform - Apache
2.x
|
This is a perfect fit for CGI problem we are seeing in #2101 and we will test your PR |
|
cPanel has shipped this patch to customers for several months now without any apparent problems. |
Apparently not. The file in the source doesn't have that patch (https://github.com/CpanelInc/mod_security2/blob/master/SOURCES/modsecurity-2.9.3.tar.gz) |
|
Although I updated #2101 at the time, last year, I will also point out here that this patch fixed our thread problem. Mod_security 2.x was not written with thread safety in mind and there was at least too much unsafe sharing of global pools between threads in an apache 2.4 event MPM. |
The patch is applied while building the RPM. The tar.gz file in this repo should be a pristine copy of the original modsecurity tarball. |
|
Indeed. |
|
Any chance to have this merged? Thanks |
|
Merged! Thanks @lightsey |
I tested the first commit fixing the re_precomp rules ( msre_op_validateHash_execute and msre_op_rx_execute ) using the example configuration I provided in issue #890.
The second commit fixing the other usage of rule->ruleset->mp in functions that are run during HTTP requests hasn't gone through any real testing. Nothing stands out to me in the code as suggesting that these should be using storage pools that are shared between all the threads in the process, but I'm not that very familiar with ModSecurity's codebase and could be mistaken.