fix: document PCRE1 global state thread-safety hazard and add build w…#380
Open
eilandert wants to merge 1 commit into
Open
fix: document PCRE1 global state thread-safety hazard and add build w…#380eilandert wants to merge 1 commit into
eilandert wants to merge 1 commit into
Conversation
…arning The PCRE1 malloc/free workaround saves and restores the global pcre_malloc and pcre_free function pointers around ModSecurity calls. These are process-global in PCRE1 and not protected by any lock. In nginx configurations that use thread pools (NGX_THREADS), concurrent requests can race on these globals: one thread may allocate memory from pool A while another swaps the pointer to pool B, causing the subsequent free to operate on the wrong pool and corrupt heap memory. Changes: - Expand the block comment to clearly describe the thread-safety hazard and the conditions under which it can manifest - Add a #warning directive that fires at compile time when building with both PCRE1 (no NGX_PCRE2) and nginx thread pools (NGX_THREADS enabled), prompting the operator to switch to PCRE2 - No logic change; the runtime behaviour is unchanged for the common event-loop (non-threaded) case The correct long-term fix is to build nginx with PCRE2. PCRE2 does not use global allocator pointers and eliminates this issue entirely. Severity: High (thread-unsafe global state under NGX_THREADS builds) Reported-by: Security audit 2026-05-13
|
There was a problem hiding this comment.
Pull request overview
This PR documents the PCRE1 allocator global-state hazard in threaded nginx builds and adds a compile-time warning for PCRE1 plus NGX_THREADS configurations.
Changes:
- Expanded the PCRE workaround comment to describe thread-safety risks.
- Added a preprocessor warning for PCRE1 builds with nginx thread support enabled.
- Clarified that PCRE2 avoids the global allocator pointer issue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * via another pool, causing heap corruption. | ||
| * | ||
| * This block is compiled only when building against PCRE1 (not PCRE2). | ||
| * Building nginx with PCRE2 (--with-pcre2 or linking against libpcre2) avoids |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…arning
The PCRE1 malloc/free workaround saves and restores the global pcre_malloc and pcre_free function pointers around ModSecurity calls. These are process-global in PCRE1 and not protected by any lock.
In nginx configurations that use thread pools (NGX_THREADS), concurrent requests can race on these globals: one thread may allocate memory from pool A while another swaps the pointer to pool B, causing the subsequent free to operate on the wrong pool and corrupt heap memory.
Changes:
The correct long-term fix is to build nginx with PCRE2. PCRE2 does not use global allocator pointers and eliminates this issue entirely.