Skip to content

test: add valgrind memcheck/helgrind soak of the running module - #93

Open
fzipi wants to merge 4 commits into
owasp-modsecurity:masterfrom
fzipi:test/valgrind-soak
Open

test: add valgrind memcheck/helgrind soak of the running module#93
fzipi wants to merge 4 commits into
owasp-modsecurity:masterfrom
fzipi:test/valgrind-soak

Conversation

@fzipi

@fzipi fzipi commented Jul 25, 2026

Copy link
Copy Markdown

Stacks on #92 (Docker test environment) -- this branch is based on test/docker-build-environment, so the diff here is just the commit below until #92 merges.

Summary

  • Add tools/soak.sh, adapted from coraza-nginx's tools/soak.sh: drives a real httpd (with the connector loaded) under valgrind memcheck or helgrind with concurrent benign and attack-shaped traffic for a fixed duration
  • Unlike the nginx version, this soak also periodically issues a graceful restart (SIGUSR1) against the running master while traffic is in flight -- that's the exact operation issue apache graceful restart + Apache connector + rules = memory leak #82 reports leaking memory on, so the soak directly exercises it rather than just idling
  • Add Dockerfile.fuzz, kept separate from the main Dockerfile per review feedback: it layers valgrind + curl on top of the already-built modsec3-apache-test image (docker build -t modsec3-apache-test . then docker build -f Dockerfile.fuzz -t modsec3-soak .) instead of duplicating the build
  • Add .github/workflows/soak.yml: manual (workflow_dispatch) + weekly scheduled only, not on every PR -- a valgrind soak runs 10-50x slower than native, and see below, this connector currently has open leaks the soak is expected to (correctly) fail on
  • Add tools/valgrind.suppress, a starter suppressions file with instructions for adding entries once confirmed genuine (APR pools free everything at pool-destruction time, so "still reachable" is expected noise; only "definitely lost" and tool error counts fail the soak)

What the soak already found

Running it locally (USE_VALGRIND=1) reproduces issue #82: every graceful restart leaks a rules_set --

5,549 (1,640 direct, 3,909 indirect) bytes in 1 blocks are definitely lost
   at operator new(unsigned long)
   by msc_create_rules_set (rules_set.cc:278)
   by msc_hook_create_config_directory
   by ap_single_module_configure
   ...

one such block per SIGUSR1, confirming the root cause is a rules_set never freed when the module re-reads its config on restart.

It also found a second, separate leak: an unbounded per-request leak in modsecurity::Transaction::intervention() (transaction.cc:1382) -- the intervention log message is strdup'd and never freed, growing with every blocked request (200KB+ per batch in a short run).

The helgrind soak runs clean on the connector's own code; its two findings (pthread_mutex_destroy at exit in libp11-kit, and a scoreboard write race in httpd core's update_child_status_internal) are pre-existing httpd-core/runtime noise, not this module.

I have not fixed either leak in this PR -- it's scoped to adding the tooling that surfaces them. Happy to open a follow-up for the fixes if wanted.

Test plan

  • docker build -t modsec3-apache-test . then docker build -f Dockerfile.fuzz -t modsec3-soak .
  • Plain run (no valgrind) passes cleanly, including a graceful restart mid-soak
  • USE_VALGRIND=1 run correctly fails and reports the two leaks above with full stack traces
  • USE_HELGRIND=1 run correctly completes and reports only pre-existing httpd-core/runtime noise
  • shellcheck/shfmt clean on tools/soak.sh; actionlint/zizmor clean on the new workflow

fzipi and others added 4 commits July 25, 2026 15:49
Adds a multi-stage Dockerfile that builds libmodsecurity v3, Apache
2.4.62, and the connector, plus docker-compose.yml, an automated
test-connector.sh smoke test, and docs summarizing the fixes and how
to verify them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test-rules.conf's REQUEST_BODY rule used status:488, a non-standard
code Apache can't emit on the wire (it falls back to 500), while
test-connector.sh inconsistently expected 403 for the same rule in one
test and 488 in another. Use 403 everywhere so the smoke suite
actually passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds the Docker image and runs test-connector.sh on push to master
and on pull requests that touch the Dockerfile, docker-compose.yml,
test-connector.sh, or src/. There was previously no CI covering the
Docker build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds tools/soak.sh, adapted from coraza-nginx's tools/soak.sh: drives a
real httpd (built from the existing Dockerfile) under memcheck or
helgrind with concurrent benign and attack-shaped traffic, while
periodically issuing a graceful restart (SIGUSR1) -- the exact
operation issue owasp-modsecurity#82 reports leaking memory -- then asserts no
leak/race/crash and that WAF verdicts held.

Dockerfile.fuzz is kept separate from the main Dockerfile: it layers
valgrind and curl on top of the already-built modsec3-apache-test
image rather than duplicating its build steps.

A manual/scheduled-only workflow (.github/workflows/soak.yml) runs
this; it is not wired into the on-PR build since a soak under valgrind
runs 10-50x slower and this connector has known open leaks, so the job
is expected to fail until those are fixed. Confirmed locally: the
memcheck soak reproduces issue owasp-modsecurity#82 (a rules_set leaked on every
graceful restart, via msc_create_rules_set) and additionally finds an
unbounded per-request leak in ModSecurity::Transaction::intervention's
strdup'd message; the helgrind soak runs cleanly on the connector's own
code (its two findings are in httpd core / libp11-kit, not this
module).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fzipi fzipi changed the title Add valgrind memcheck/helgrind soak of the running module test: add valgrind memcheck/helgrind soak of the running module Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant