-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
msr_global_mutex_lock: handle errors from apr_global_mutex_lock #3257
msr_global_mutex_lock: handle errors from apr_global_mutex_lock #3257
Conversation
#3255 acknowledged to be solved by the PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Before we merge, I want to check why the lock acquire was failed (see #3255 - the user reported that this issue came with the new version where we introduced this method). |
The problem was already happening in the previous versions. Looks to me linked to concurrency. |
Co-authored-by: Max Leske <[email protected]>
Co-authored-by: Max Leske <[email protected]>
May be - but now the audit.log is empty (with this patch too). I think we should investigate this issue. I already installed FreeBSD and could reproduce the behavior, so I'm on it. |
the audit.log is empty, but you should have an entry in the error log (about the problem at creation time) |
Indeed, but I think we should investigate the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments.
…stern/ModSecurity into v2/pr/msr_global_mutex_lock
|
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
I propose to accept this PR first, then create one that doesn't use the temp filename, as it's not the right way to create a mutx, see https://lists.apache.org/thread/ykb26kg4lgcqnldvxwd9p6hv16fy4z9l |
Okay, I approved this PR and resolved all conversation. You can merge this PR now. |
Implemented all requested changes but a comment
apr_global_mutex_lock
is sometimes called with a lock that wasn't created (for any reason).In this case, the pointer is null and
apr_global_mutex_lock
dereferences a null pointer, leading to a crash.This PR creates a wrapper around
apr_global_mutex_lock
to handle checking that and correct logging.Same for
msr_global_mutex_unlock
.