msr_global_mutex_lock: Handle errors from apr_global_mutex_lock

owasp-modsecurity · Sep 12, 2024 · b522010 · b522010
1 parent 0e6fc62
commit b522010
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 58 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,