Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 090e4d3

Browse files
author
Marc Stern
authored
Merge pull request #3257 from marcstern/v2/pr/msr_global_mutex_lock
msr_global_mutex_lock: handle errors from apr_global_mutex_lock
2 parents d6f1ebb + c99d931 commit 090e4d3

File tree

6 files changed

+74
-100
lines changed

6 files changed

+74
-100
lines changed

‎CHANGES‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
(to be released) - 2.9.x
2+
------------------------
3+
4+
* handle errors from apr_global_mutex_lock
5+
[PR #3257 - @marcstern]
6+
17
03 Sep 2024 - 2.9.8
28
-------------------
39

‎apache2/modsecurity.c‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp) {
126126
apr_status_t rc;
127127
apr_file_t *lock_name;
128128
const char *temp_dir;
129-
const char *filename;
129+
const char *filename=NULL;
130130

131131
// get platform temp dir
132132
rc = apr_temp_dir_get(&temp_dir, mp);
@@ -166,6 +166,41 @@ int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp) {
166166
#endif /* MSC_TEST */
167167
return APR_SUCCESS;
168168
}
169+
170+
/**
171+
* handle errors from apr_global_mutex_lock
172+
*/
173+
int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct) {
174+
assert(msr);
175+
assert(msr->modsecurity); // lock is msr->modsecurity->..._lock
176+
assert(msr->mp);
177+
if (!lock) {
178+
msr_log(msr, 1, "%s: Global mutex was not created", fct);
179+
return -1;
180+
}
181+
182+
int rc = apr_global_mutex_lock(lock);
183+
if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s", get_apr_error(msr->mp, rc));
184+
return rc;
185+
}
186+
/**
187+
* handle errors from apr_global_mutex_unlock
188+
*/
189+
int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct) {
190+
assert(msr);
191+
assert(msr->modsecurity); // lock is msr->modsecurity->..._lock
192+
assert(msr->mp);
193+
if (!lock) {
194+
msr_log(msr, 1, "%s: Global mutex was not created", fct);
195+
return -1;
196+
}
197+
198+
int rc = apr_global_mutex_unlock(lock);
199+
// We should have get the warning at lock time, so ignore it here
200+
// if (rc != APR_SUCCESS) msr_log(msr, 1, "Audit log: Failed to unlock global mutex: %s", get_apr_error(msr->mp, rc));
201+
return rc;
202+
}
203+
169204
/**
170205
* Initialise the modsecurity engine. This function must be invoked
171206
* after configuration processing is complete as Apache needs to know the

‎apache2/modsecurity.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,8 @@ struct msc_parm {
707707

708708
/* Reusable functions */
709709
int acquire_global_lock(apr_global_mutex_t **lock, apr_pool_t *mp);
710+
int msr_global_mutex_lock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct);
711+
int msr_global_mutex_unlock(modsec_rec* msr, apr_global_mutex_t* lock, const char* fct);
710712

711713
/* Engine functions */
712714

‎apache2/msc_geo.c‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
325325
msr_log(msr, 9, "GEO: Using address \"%s\" (0x%08lx). %lu", targetip, ipnum, ipnum);
326326
}
327327

328-
ret = apr_global_mutex_lock(msr->modsecurity->geo_lock);
329-
if (ret != APR_SUCCESS) {
330-
msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
331-
get_apr_error(msr->mp, ret));
332-
}
328+
msr_global_mutex_lock(msr, msr->modsecurity->geo_lock, "Geo lookup");
333329

334330
for (level = 31; level >= 0; level--) {
335331
/* Read the record */
@@ -361,13 +357,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
361357
if (rec_val == geo->ctry_offset) {
362358
*error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\").", log_escape(msr->mp, target));
363359
msr_log(msr, 4, "%s", *error_msg);
364-
365-
ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
366-
if (ret != APR_SUCCESS) {
367-
msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
368-
get_apr_error(msr->mp, ret));
369-
}
370-
360+
msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
371361
return 0;
372362
}
373363

@@ -377,13 +367,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
377367
if ((country <= 0) || (country > GEO_COUNTRY_LAST)) {
378368
*error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\" (country %d).", log_escape(msr->mp, target), country);
379369
msr_log(msr, 4, "%s", *error_msg);
380-
381-
ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
382-
if (ret != APR_SUCCESS) {
383-
msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
384-
get_apr_error(msr->mp, ret));
385-
}
386-
370+
msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
387371
return 0;
388372
}
389373

@@ -408,13 +392,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
408392
if ((country <= 0) || (country > GEO_COUNTRY_LAST)) {
409393
*error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\" (country %d).", log_escape(msr->mp, target), country);
410394
msr_log(msr, 4, "%s", *error_msg);
411-
412-
ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
413-
if (ret != APR_SUCCESS) {
414-
msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
415-
get_apr_error(msr->mp, ret));
416-
}
417-
395+
msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
418396
return 0;
419397
}
420398
if (msr->txcfg->debuglog_level >= 9) {
@@ -503,13 +481,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro
503481
}
504482

505483
*error_msg = apr_psprintf(msr->mp, "Geo lookup for \"%s\" succeeded.", log_escape(msr->mp, target));
506-
507-
ret = apr_global_mutex_unlock(msr->modsecurity->geo_lock);
508-
if (ret != APR_SUCCESS) {
509-
msr_log(msr, 1, "Geo Lookup: Failed to lock proc mutex: %s",
510-
get_apr_error(msr->mp, ret));
511-
}
512-
484+
msr_global_mutex_unlock(msr, msr->modsecurity->geo_lock, "Geo Lookup");
513485
return 1;
514486
}
515487

‎apache2/msc_logging.c‎

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -757,14 +757,7 @@ void sec_audit_logger_json(modsec_rec *msr) {
757757

758758
/* Lock the mutex, but only if we are using serial format. */
759759
if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
760-
if (!msr->modsecurity->auditlog_lock) msr_log(msr, 1, "Audit log: Global mutex was not created");
761-
else {
762-
rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
763-
if (rc != APR_SUCCESS) {
764-
msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s",
765-
get_apr_error(msr->mp, rc));
766-
}
767-
}
760+
msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
768761
}
769762

770763
/**
@@ -1471,15 +1464,8 @@ void sec_audit_logger_json(modsec_rec *msr) {
14711464
* as it does not need an index file.
14721465
*/
14731466
if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
1474-
14751467
/* Unlock the mutex we used to serialise access to the audit log file. */
1476-
rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
1477-
if (rc != APR_SUCCESS) {
1478-
msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
1479-
apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
1480-
get_apr_error(msr->mp, rc));
1481-
}
1482-
1468+
msr_global_mutex_unlock(msr, msr->modsecurity->auditlog_lock, "Audit log");
14831469
return;
14841470
}
14851471

@@ -1650,11 +1636,7 @@ void sec_audit_logger_native(modsec_rec *msr) {
16501636

16511637
/* Lock the mutex, but only if we are using serial format. */
16521638
if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
1653-
rc = apr_global_mutex_lock(msr->modsecurity->auditlog_lock);
1654-
if (rc != APR_SUCCESS) {
1655-
msr_log(msr, 1, "Audit log: Failed to lock global mutex: %s",
1656-
get_apr_error(msr->mp, rc));
1657-
}
1639+
msr_global_mutex_lock(msr, msr->modsecurity->auditlog_lock, "Audit log");
16581640
}
16591641

16601642

@@ -2253,15 +2235,8 @@ void sec_audit_logger_native(modsec_rec *msr) {
22532235
*/
22542236
if (msr->txcfg->auditlog_type != AUDITLOG_CONCURRENT) {
22552237
sec_auditlog_write(msr, "\n", 1);
2256-
22572238
/* Unlock the mutex we used to serialise access to the audit log file. */
2258-
rc = apr_global_mutex_unlock(msr->modsecurity->auditlog_lock);
2259-
if (rc != APR_SUCCESS) {
2260-
msr_log(msr, 1, "Audit log: Failed to unlock global mutex '%s': %s",
2261-
apr_global_mutex_lockfile(msr->modsecurity->auditlog_lock),
2262-
get_apr_error(msr->mp, rc));
2263-
}
2264-
2239+
msr_global_mutex_unlock(msr, msr->modsecurity->auditlog_lock, "Audit log");
22652240
return;
22662241
}
22672242

‎apache2/persist_dbm.c‎

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -125,19 +125,15 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
125125

126126
if (existing_dbm == NULL) {
127127
#ifdef GLOBAL_COLLECTION_LOCK
128-
rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
129-
if (rc != APR_SUCCESS) {
130-
msr_log(msr, 1, "collection_retrieve_ex: Failed to lock proc mutex: %s",
131-
get_apr_error(msr->mp, rc));
132-
goto cleanup;
133-
}
128+
rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
129+
if (rc != APR_SUCCESS) goto cleanup;
134130
#endif
135131
rc = apr_sdbm_open(&dbm, dbm_filename, APR_READ | APR_SHARELOCK,
136132
CREATEMODE, msr->mp);
137133
if (rc != APR_SUCCESS) {
138134
dbm = NULL;
139135
#ifdef GLOBAL_COLLECTION_LOCK
140-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
136+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
141137
#endif
142138
goto cleanup;
143139
}
@@ -173,7 +169,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
173169
if (existing_dbm == NULL) {
174170
apr_sdbm_close(dbm);
175171
#ifdef GLOBAL_COLLECTION_LOCK
176-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
172+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
177173
#endif
178174
dbm = NULL;
179175
}
@@ -222,12 +218,8 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
222218
if (apr_table_get(col, "KEY") == NULL) {
223219
if (existing_dbm == NULL) {
224220
#ifdef GLOBAL_COLLECTION_LOCK
225-
rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
226-
if (rc != APR_SUCCESS) {
227-
msr_log(msr, 1, "collection_retrieve_ex: Failed to lock proc mutex: %s",
228-
get_apr_error(msr->mp, rc));
229-
goto cleanup;
230-
}
221+
rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
222+
if (rc != APR_SUCCESS) goto cleanup;
231223
#endif
232224
rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
233225
CREATEMODE, msr->mp);
@@ -236,7 +228,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
236228
log_escape(msr->mp, dbm_filename), get_apr_error(msr->mp, rc));
237229
dbm = NULL;
238230
#ifdef GLOBAL_COLLECTION_LOCK
239-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
231+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
240232
#endif
241233
goto cleanup;
242234
}
@@ -261,7 +253,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
261253
if (existing_dbm == NULL) {
262254
apr_sdbm_close(dbm);
263255
#ifdef GLOBAL_COLLECTION_LOCK
264-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
256+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
265257
#endif
266258
dbm = NULL;
267259
}
@@ -326,7 +318,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
326318

327319
apr_sdbm_close(dbm);
328320
#ifdef GLOBAL_COLLECTION_LOCK
329-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
321+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
330322
#endif
331323
}
332324

@@ -337,7 +329,7 @@ static apr_table_t *collection_retrieve_ex(apr_sdbm_t *existing_dbm, modsec_rec
337329
if ((existing_dbm == NULL) && dbm) {
338330
apr_sdbm_close(dbm);
339331
#ifdef GLOBAL_COLLECTION_LOCK
340-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
332+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_retrieve_ex");
341333
#endif
342334
}
343335

@@ -408,12 +400,8 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
408400

409401
#ifdef GLOBAL_COLLECTION_LOCK
410402
/* Need to lock to pull in the stored data again and apply deltas. */
411-
rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
412-
if (rc != APR_SUCCESS) {
413-
msr_log(msr, 1, "collection_store: Failed to lock proc mutex: %s",
414-
get_apr_error(msr->mp, rc));
415-
goto error;
416-
}
403+
int ret = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collection_store");
404+
if (ret != APR_SUCCESS) goto error;
417405
#endif
418406

419407
/* Delete IS_NEW on store. */
@@ -473,7 +461,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
473461
CREATEMODE, msr->mp);
474462
if (rc != APR_SUCCESS) {
475463
#ifdef GLOBAL_COLLECTION_LOCK
476-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
464+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
477465
#endif
478466
msr_log(msr, 1, "collection_store: Failed to access DBM file \"%s\": %s", log_escape(msr->mp, dbm_filename),
479467
get_apr_error(msr->mp, rc));
@@ -556,7 +544,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
556544
if (dbm != NULL) {
557545
#ifdef GLOBAL_COLLECTION_LOCK
558546
apr_sdbm_close(dbm);
559-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
547+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
560548
#else
561549
apr_sdbm_unlock(dbm);
562550
apr_sdbm_close(dbm);
@@ -619,7 +607,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
619607
if (dbm != NULL) {
620608
#ifdef GLOBAL_COLLECTION_LOCK
621609
apr_sdbm_close(dbm);
622-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
610+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
623611
#else
624612
apr_sdbm_unlock(dbm);
625613
apr_sdbm_close(dbm);
@@ -631,7 +619,7 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
631619

632620
#ifdef GLOBAL_COLLECTION_LOCK
633621
apr_sdbm_close(dbm);
634-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
622+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collection_store");
635623
#else
636624
apr_sdbm_unlock(dbm);
637625
apr_sdbm_close(dbm);
@@ -684,19 +672,15 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
684672
}
685673

686674
#ifdef GLOBAL_COLLECTION_LOCK
687-
rc = apr_global_mutex_lock(msr->modsecurity->dbm_lock);
688-
if (rc != APR_SUCCESS) {
689-
msr_log(msr, 1, "collections_remove_stale: Failed to lock proc mutex: %s",
690-
get_apr_error(msr->mp, rc));
691-
goto error;
692-
}
675+
rc = msr_global_mutex_lock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
676+
if (rc != APR_SUCCESS) goto error;
693677
#endif
694678

695679
rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
696680
CREATEMODE, msr->mp);
697681
if (rc != APR_SUCCESS) {
698682
#ifdef GLOBAL_COLLECTION_LOCK
699-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
683+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
700684
#endif
701685
msr_log(msr, 1, "collections_remove_stale: Failed to access DBM file \"%s\": %s", log_escape(msr->mp, dbm_filename),
702686
get_apr_error(msr->mp, rc));
@@ -799,7 +783,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
799783

800784
apr_sdbm_close(dbm);
801785
#ifdef GLOBAL_COLLECTION_LOCK
802-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
786+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
803787
#endif
804788
return 1;
805789

@@ -808,7 +792,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
808792
if (dbm) {
809793
apr_sdbm_close(dbm);
810794
#ifdef GLOBAL_COLLECTION_LOCK
811-
apr_global_mutex_unlock(msr->modsecurity->dbm_lock);
795+
msr_global_mutex_unlock(msr, msr->modsecurity->dbm_lock, "collections_remove_stale");
812796
#endif
813797
}
814798

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /