From b52201010d4fb450ac36cd96c352f0f8f153f1b5 Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Thu, 12 Sep 2024 12:18:25 +0200
Subject: [PATCH 1/8] msr_global_mutex_lock: Handle errors from
 apr_global_mutex_lock

---
 apache2/modsecurity.c | 18 ++++++++++++++++++
 apache2/modsecurity.h |  1 +
 apache2/msc_geo.c     |  6 +-----
 apache2/msc_logging.c | 33 ++++-----------------------------
 apache2/persist_dbm.c | 32 ++++++++------------------------
 5 files changed, 32 insertions(+), 58 deletions(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index 55150afe2..7594bdc25 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -166,6 +166,24 @@ int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp) {
 #endif /* MSC_TEST */
     return APR_SUCCESS;
 }
+
+/**
+ * handle errors from apr_global_mutex_lock
+ */
+int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct) {
+    assert(msr);
+    assert(msr->modsecurity); // lock is msr->modsecurity->..._lock
+    assert(msr->mp);
+    if (!lock) {
+        msr_log(msr, 1, "%s: Global mutex was not created", fct);
+        return -1;
+    }
+
+    int rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
+    if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
+    return rc;
+}
+
 /**
  * Initialise the modsecurity engine. This function must be invoked
  * after configuration processing is complete as Apache needs to know the
diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h
index d1aa1d834..4a3ef5427 100644
--- a/apache2/modsecurity.h
+++ b/apache2/modsecurity.h
@@ -707,6 +707,7 @@ struct msc_parm {
 
 /* Reusable functions */
 int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp);
+int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct);
 
 /* Engine functions */
 
diff --git a/apache2/msc_geo.c b/apache2/msc_geo.c
index 6f60b0032..f644737cd 100644
--- a/apache2/msc_geo.c
+++ b/apache2/msc_geo.c
@@ -325,11 +325,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
         msr_log(msr, 9, "GEO: Using address \"%s\" (0x%08lx). %lu", targetip, ipnum, ipnum);
     }
 
-    ret = apr_global_mutex_lock(msr->modsecurity->geo_lock);
-    if (ret != APR_SUCCESS) {
-        msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
-                get_apr_error(msr->mp, ret));
-    }
+    msr_global_mutex_lock(msr, msr->modsecurity->geo_lock, "Geo lookup");
 
     for (level = 31; level >= 0; level--) {
         /* Read the record */
diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c
index 46c088262..8fd7e8ee7 100644
--- a/apache2/msc_logging.c
+++ b/apache2/msc_logging.c
@@ -757,14 +757,7 @@ void sec_audit_logger_json(modsec_rec *msr) {
 
     /* Lock the mutex, but only if we are using serial format. */
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
-        if (!msr->modsecurity->auditlog_lock) msr_log(msr, 1, "Audit log: Global mutex was not created");
-        else {
-            rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
-            if (rc != APR_SUCCESS) {
-                msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s",
-                    get_apr_error(msr->mp, rc));
-            }
-        }
+        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
     }
 
     /**
@@ -1471,15 +1464,8 @@ void sec_audit_logger_json(modsec_rec *msr) {
      * as it does not need an index file.
      */
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
-
         /* Unlock the mutex we used to serialise access to the audit log file. */
-        rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
-        if (rc != APR_SUCCESS) {
-            msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
-                    apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
-                    get_apr_error(msr->mp, rc));
-        }
-
+        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
         return;
     }
 
@@ -1650,11 +1636,7 @@ void sec_audit_logger_native(modsec_rec *msr) {
 
     /* Lock the mutex, but only if we are using serial format. */
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
-        rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
-        if (rc != APR_SUCCESS) {
-            msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s",
-                get_apr_error(msr->mp, rc));
-        }
+        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
     }
 
 
@@ -2253,15 +2235,8 @@ void sec_audit_logger_native(modsec_rec *msr) {
      */
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
         sec_auditlog_write(msr, "\n", 1);
-
         /* Unlock the mutex we used to serialise access to the audit log file. */
-        rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
-        if (rc != APR_SUCCESS) {
-            msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
-                    apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
-                    get_apr_error(msr->mp, rc));
-        }
-
+        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
         return;
     }
 
diff --git a/apache2/persist_dbm.c b/apache2/persist_dbm.c
index c685e1de3..b1a2f17e8 100644
--- a/apache2/persist_dbm.c
+++ b/apache2/persist_dbm.c
@@ -125,12 +125,8 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
 
     if (existing_dbm == NULL) {
 #ifdef GLOBAL_COLLECTION_LOCK
-        rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
-        if (rc != APR_SUCCESS) {
-            msr_log(msr, 1, "collection_retrieve_ex: Failed to lock proc mutex: %s",
-                    get_apr_error(msr->mp, rc));
-            goto cleanup;
-        }
+        rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
+        if (rc != APR_SUCCESS) goto cleanup;
 #endif
         rc = apr_sdbm_open(&dbm, dbm_filename, APR_READ | APR_SHARELOCK,
             CREATEMODE, msr->mp);
@@ -222,12 +218,8 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
     if (apr_table_get(col, "KEY") == NULL) {
         if (existing_dbm == NULL) {
 #ifdef GLOBAL_COLLECTION_LOCK
-            rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
-            if (rc != APR_SUCCESS) {
-                msr_log(msr, 1, "collection_retrieve_ex: Failed to lock proc mutex: %s",
-                        get_apr_error(msr->mp, rc));
-                goto cleanup;
-            }
+            rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
+            if (rc != APR_SUCCESS) goto cleanup;
 #endif
             rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
                 CREATEMODE, msr->mp);
@@ -408,12 +400,8 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
 
 #ifdef GLOBAL_COLLECTION_LOCK
     /* Need to lock to pull in the stored data again and apply deltas. */
-    rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
-    if (rc != APR_SUCCESS) {
-        msr_log(msr, 1, "collection_store: Failed to lock proc mutex: %s",
-                get_apr_error(msr->mp, rc));
-        goto error;
-    }
+    int ret = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_store");
+    if (ret != APR_SUCCESS) goto error;
 #endif
 
     /* Delete IS_NEW on store. */
@@ -684,12 +672,8 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
     }
 
 #ifdef GLOBAL_COLLECTION_LOCK
-    rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
-    if (rc != APR_SUCCESS) {
-        msr_log(msr, 1, "collections_remove_stale: Failed to lock proc mutex: %s",
-                get_apr_error(msr->mp, rc));
-        goto error;
-    }
+    rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
+    if (rc != APR_SUCCESS) goto error;
 #endif
 
     rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,

From 449c080e63b17558cb609aa9cec323227bd24a0f Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Thu, 12 Sep 2024 13:01:44 +0200
Subject: [PATCH 2/8] Same for global_mutex_unlock

---
 apache2/modsecurity.c | 16 ++++++++++++++++
 apache2/modsecurity.h |  1 +
 apache2/msc_geo.c     | 32 ++++----------------------------
 apache2/persist_dbm.c | 26 +++++++++++++-------------
 4 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index 7594bdc25..9439c4477 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -183,6 +183,22 @@ int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char*
     if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
     return rc;
 }
+/**
+ * handle errors from apr_global_mutex_unlock
+ */
+int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct) {
+    assert(msr);
+    assert(msr->modsecurity); // lock is msr->modsecurity->..._lock
+    assert(msr->mp);
+    if (!lock) {
+        msr_log(msr, 1, "%s: Global mutex was not created", fct);
+        return -1;
+    }
+
+    int rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
+    if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
+    return rc;
+}
 
 /**
  * Initialise the modsecurity engine. This function must be invoked
diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h
index 4a3ef5427..a1751000b 100644
--- a/apache2/modsecurity.h
+++ b/apache2/modsecurity.h
@@ -708,6 +708,7 @@ struct msc_parm {
 /* Reusable functions */
 int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp);
 int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct);
+int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct);
 
 /* Engine functions */
 
diff --git a/apache2/msc_geo.c b/apache2/msc_geo.c
index f644737cd..7d3a8e402 100644
--- a/apache2/msc_geo.c
+++ b/apache2/msc_geo.c
@@ -357,13 +357,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
     if (rec_val == geo->ctry_offset) {
         *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\").", log_escape(msr->mp, target));
         msr_log(msr, 4, "%s", *error_msg);
-
-        ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
-        if (ret != APR_SUCCESS) {
-            msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
-                    get_apr_error(msr->mp, ret));
-        }
-
+        msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
         return 0;
     }
 
@@ -373,13 +367,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
         if ((country <= 0) || (country > GEO_COUNTRY_LAST)) {
             *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\" (country %d).", log_escape(msr->mp, target), country);
             msr_log(msr, 4, "%s", *error_msg);
-
-            ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
-            if (ret != APR_SUCCESS) {
-                msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
-                        get_apr_error(msr->mp, ret));
-            }
-
+            msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
             return 0;
         }
 
@@ -404,13 +392,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
         if ((country <= 0) || (country > GEO_COUNTRY_LAST)) {
             *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\" (country %d).", log_escape(msr->mp, target), country);
             msr_log(msr, 4, "%s", *error_msg);
-
-            ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
-            if (ret != APR_SUCCESS) {
-                msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
-                        get_apr_error(msr->mp, ret));
-            }
-
+            msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
             return 0;
         }
         if (msr->txcfg->debuglog_level >= 9) {
@@ -499,13 +481,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
     }
 
     *error_msg = apr_psprintf(msr->mp, "Geo lookup for \"%s\" succeeded.", log_escape(msr->mp, target));
-
-    ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
-    if (ret != APR_SUCCESS) {
-        msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
-                get_apr_error(msr->mp, ret));
-    }
-
+    msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
     return 1;
 }
 
diff --git a/apache2/persist_dbm.c b/apache2/persist_dbm.c
index b1a2f17e8..ba8475cc5 100644
--- a/apache2/persist_dbm.c
+++ b/apache2/persist_dbm.c
@@ -133,7 +133,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
         if (rc != APR_SUCCESS) {
             dbm = NULL;
 #ifdef GLOBAL_COLLECTION_LOCK
-            apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+            msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
             goto cleanup;
         }
@@ -169,7 +169,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
     if (existing_dbm == NULL) {
         apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
         dbm = NULL;
     }
@@ -228,7 +228,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
                     log_escape(msr->mp, dbm_filename), get_apr_error(msr->mp, rc));
                 dbm = NULL;
 #ifdef GLOBAL_COLLECTION_LOCK
-                apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+                msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
                 goto cleanup;
             }
@@ -253,7 +253,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
         if (existing_dbm == NULL) {
             apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-            apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+            msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
             dbm = NULL;
         }
@@ -318,7 +318,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
 
         apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
     }
 
@@ -329,7 +329,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
     if ((existing_dbm == NULL) && dbm) {
         apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
 #endif
     }
 
@@ -461,7 +461,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
         CREATEMODE, msr->mp);
     if (rc != APR_SUCCESS) {
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
 #endif
         msr_log(msr, 1, "collection_store: Failed to access DBM file \"%s\": %s", log_escape(msr->mp, dbm_filename),
             get_apr_error(msr->mp, rc));
@@ -544,7 +544,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
         if (dbm != NULL) {
 #ifdef GLOBAL_COLLECTION_LOCK
             apr_sdbm_close(dbm);
-            apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+            msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
 #else
             apr_sdbm_unlock(dbm);
             apr_sdbm_close(dbm);
@@ -607,7 +607,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
         if (dbm != NULL) {
 #ifdef GLOBAL_COLLECTION_LOCK
             apr_sdbm_close(dbm);
-            apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+            msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
 #else
             apr_sdbm_unlock(dbm);
             apr_sdbm_close(dbm);
@@ -619,7 +619,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
 
 #ifdef GLOBAL_COLLECTION_LOCK
     apr_sdbm_close(dbm);
-    apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+    msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
 #else
     apr_sdbm_unlock(dbm);
     apr_sdbm_close(dbm);
@@ -680,7 +680,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
             CREATEMODE, msr->mp);
     if (rc != APR_SUCCESS) {
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
 #endif
         msr_log(msr, 1, "collections_remove_stale: Failed to access DBM file \"%s\": %s", log_escape(msr->mp, dbm_filename),
                 get_apr_error(msr->mp, rc));
@@ -783,7 +783,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
 
     apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-    apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+    msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
 #endif
     return 1;
 
@@ -792,7 +792,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
     if (dbm) {
         apr_sdbm_close(dbm);
 #ifdef GLOBAL_COLLECTION_LOCK
-        apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
+        msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
 #endif
     }
 

From b850c74b12017153e8fdc94796228c9ff974725f Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Thu, 12 Sep 2024 14:07:55 +0200
Subject: [PATCH 3/8] We should have get the warning at lock time, so ignore it
 at unlock time

---
 apache2/modsecurity.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index 9439c4477..e31c3407d 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -196,7 +196,8 @@ int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const cha
     }
 
     int rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
-    if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
+    // We should have get the warning at lock time, so ignore it here
+    // if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", get_apr_error(msr->mp, rc));
     return rc;
 }
 

From bd54f01cd3e93eac32efb771764dec9b7539fbeb Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Fri, 20 Sep 2024 12:51:03 +0200
Subject: [PATCH 4/8] Added CHANGES

---
 CHANGES | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGES b/CHANGES
index 8a35b6752..7bc8cbd27 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,9 @@
+(to be released) - 2.9.x 
+------------------------
+
+ * handle errors from apr_global_mutex_lock
+   [PR #3257 - @marcstern]
+
 03 Sep 2024 - 2.9.8
 -------------------
 

From 339d6df2a566bc3856b6b2effd77333d46d56ada Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Fri, 27 Sep 2024 09:26:58 +0200
Subject: [PATCH 5/8] Updated comment

Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
---
 apache2/modsecurity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index e31c3407d..bb4608322 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -196,7 +196,7 @@ int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const cha
     }
 
     int rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
-    // We should have get the warning at lock time, so ignore it here
+    // We should have been warnend during lock acquisition already, so don't log the same warning here
     // if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", get_apr_error(msr->mp, rc));
     return rc;
 }

From 95dc5944d4d2371446f08b8376b321a34881a14c Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Fri, 27 Sep 2024 09:27:29 +0200
Subject: [PATCH 6/8] Updated log message

Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
---
 apache2/modsecurity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index bb4608322..e41bba9f4 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -175,7 +175,7 @@ int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char*
     assert(msr->modsecurity); // lock is msr->modsecurity->..._lock
     assert(msr->mp);
     if (!lock) {
-        msr_log(msr, 1, "%s: Global mutex was not created", fct);
+        msr_log(msr, 1, "%s: Global mutex not initialised", fct);
         return -1;
     }
 

From b8e8e30730c17ebf8eabcdeebe030e6859bddb37 Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Mon, 30 Sep 2024 13:12:38 +0200
Subject: [PATCH 7/8] Fixed parameters/functions names

---
 apache2/modsecurity.c | 4 ++--
 apache2/msc_logging.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index e31c3407d..89cab90a5 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -179,7 +179,7 @@ int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char*
         return -1;
     }
 
-    int rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
+    int rc = apr_global_mutex_lock(lock);
     if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
     return rc;
 }
@@ -195,7 +195,7 @@ int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const cha
         return -1;
     }
 
-    int rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
+    int rc = apr_global_mutex_unlock(lock);
     // We should have get the warning at lock time, so ignore it here
     // if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", get_apr_error(msr->mp, rc));
     return rc;
diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c
index 8fd7e8ee7..39588b10f 100644
--- a/apache2/msc_logging.c
+++ b/apache2/msc_logging.c
@@ -1465,7 +1465,7 @@ void sec_audit_logger_json(modsec_rec *msr) {
      */
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
         /* Unlock the mutex we used to serialise access to the audit log file. */
-        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
+        msr_global_mutex_unlock(msr, msr->modsecurity->auditlog_lock, "Audit log");
         return;
     }
 
@@ -2236,7 +2236,7 @@ void sec_audit_logger_native(modsec_rec *msr) {
     if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
         sec_auditlog_write(msr, "\n", 1);
         /* Unlock the mutex we used to serialise access to the audit log file. */
-        msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
+        msr_global_mutex_unlock(msr, msr->modsecurity->auditlog_lock, "Audit log");
         return;
     }
 

From c99d931f3cb1ad7ce0a319963428a8ccaf741646 Mon Sep 17 00:00:00 2001
From: Marc Stern <marc.stern@approach-cyber.com>
Date: Mon, 30 Sep 2024 13:53:31 +0200
Subject: [PATCH 8/8] Initialize filename to NULL

---
 apache2/modsecurity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c
index 89cab90a5..550318893 100644
--- a/apache2/modsecurity.c
+++ b/apache2/modsecurity.c
@@ -126,7 +126,7 @@ int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp) {
     apr_status_t rc;
     apr_file_t *lock_name;
     const char *temp_dir;
-    const char *filename;
+    const char *filename = NULL;
 
     // get platform temp dir
     rc = apr_temp_dir_get(&temp_dir, mp);