From c4401e4cafd619a934443477faa0bb673ec7558e Mon Sep 17 00:00:00 2001
From: Felipe Zipitria <felipe.zipitria@owasp.org>
Date: Tue, 13 Oct 2026 09:25:42 -0300
Subject: [PATCH] fix: close the read end of piped logs in every child

SecAuditLog, SecAuditLog2 and SecGuardianLog open their "|program"
pipes with ap_open_piped_log() while the configuration is read. httpd
closes the read end of its own piped logs in each child
(ap_logs_child_init), but that list is rebuilt after the configuration
pool is cleared, so the read ends opened by ModSecurity stay open in
every worker. A worker holding the read end of a pipe never gets EPIPE
when the logger process dies, and after a graceful restart it blocks in
write() for ever, which is the hang reported in #2822.

The read ends are now remembered when the pipes are opened and closed
from the child_init hook, mirroring what httpd does for its own logs.

Fixes #2822

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy
---
 apache2/apache2.h        |  2 ++
 apache2/apache2_config.c | 44 ++++++++++++++++++++++++++++++++++++++++
 apache2/mod_security2.c  |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/apache2/apache2.h b/apache2/apache2.h
index 87a17ed2e9..3fcb34e7ac 100644
--- a/apache2/apache2.h
+++ b/apache2/apache2.h
@@ -95,5 +95,7 @@ char DSOLOCAL *format_error_log_message(apr_pool_t *mp, error_message_t *em);
 const DSOLOCAL char *get_response_protocol(request_rec *r);
 
 
+void DSOLOCAL modsec_close_piped_log_read_fds(void);
+
 #endif
 
diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c
index f410f40ed5..305e4aa42d 100644
--- a/apache2/apache2_config.c
+++ b/apache2/apache2_config.c
@@ -1234,6 +1234,47 @@ static const char *cmd_audit_engine(cmd_parms *cmd, void *_dcfg, const char *p1)
     return NULL;
 }
 
+/**
+ * Piped logs are opened while the configuration is read, so unlike the
+ * ones httpd opens in its open_logs hook their read ends are not on the
+ * list httpd closes in every child. A child that keeps the read end of a
+ * pipe can never see EPIPE when the logger dies, and blocks in write()
+ * for ever after a graceful restart (#2822). Remember the read ends here
+ * and close them from the child_init hook.
+ */
+static apr_array_header_t *piped_log_read_fds = NULL;
+
+static apr_status_t piped_log_read_fds_cleanup(void *data) {
+    piped_log_read_fds = NULL;
+    return APR_SUCCESS;
+}
+
+static void piped_log_remember_read_fd(apr_pool_t *p, piped_log *pl) {
+    apr_file_t *fd = ap_piped_log_read_fd(pl);
+
+    if (fd == NULL) return;
+
+    if (piped_log_read_fds == NULL) {
+        piped_log_read_fds = apr_array_make(p, 4, sizeof(apr_file_t *));
+        apr_pool_cleanup_register(p, NULL, piped_log_read_fds_cleanup,
+            apr_pool_cleanup_null);
+    }
+    *(apr_file_t **)apr_array_push(piped_log_read_fds) = fd;
+}
+
+void modsec_close_piped_log_read_fds(void) {
+    int i;
+
+    if (piped_log_read_fds == NULL) return;
+
+    for (i = 0; i < piped_log_read_fds->nelts; i++) {
+        apr_file_t *fd = ((apr_file_t **)piped_log_read_fds->elts)[i];
+        if (fd != NULL) {
+            apr_file_close(fd);
+        }
+    }
+}
+
 static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1)
 {
     assert(cmd != NULL);
@@ -1253,6 +1294,7 @@ static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1)
                     pipe_name);
         }
         dcfg->auditlog_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog_name);
@@ -1297,6 +1339,7 @@ static const char *cmd_audit_log2(cmd_parms *cmd, void *_dcfg, const char *p1)
                     pipe_name);
         }
         dcfg->auditlog2_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog2_name);
@@ -1746,6 +1789,7 @@ static const char *cmd_guardian_log(cmd_parms *cmd, void *_dcfg,
                 pipe_name);
         }
         guardianlog_fd = ap_piped_log_write_fd(pipe_log);
+        piped_log_remember_read_fd(cmd->pool, pipe_log);
     }
     else {
         const char *file_name = ap_server_root_relative(cmd->pool, guardianlog_name);
diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c
index 73e424e59d..801bc011a1 100644
--- a/apache2/mod_security2.c
+++ b/apache2/mod_security2.c
@@ -844,6 +844,8 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t
  * Initialisation performed for every new child process.
  */
 static void hook_child_init(apr_pool_t *mp, server_rec *s) {
+    /* Drop the read ends of our piped logs, as httpd does for its own. */
+    modsec_close_piped_log_read_fds();
     modsecurity_child_init(modsecurity);
 }