On 10/7/25 6:04 PM, Joe Orton wrote:
> On Tue, Oct 07, 2025 at 03:54:29PM +0200, Ruediger Pluem wrote:
>>
>>
>> On 10/7/25 11:50 AM, Joe Orton wrote:
>>> On Tue, Oct 07, 2025 at 10:45:27AM +0100, Joe Orton wrote:
>>>> There should already be a global lock held by API consumer, since the
>>>> socache provider is marked with AP_SOCACHE_FLAG_NOTMPSAFE. It looks like
>>>> mod_authn_socache is protecting entry to the socache provider inside a
>>>> global mutex without checking that flag - overkill but should be safe.
>>>
>>> Ah, not true, it only does it for ->store(). The ->retrieve() calls are
>>> then unsafe and racy, that's definitely a bug.
>>
>> Your proposal would be to setup check_password in mod_authn_socache in a
>> similar way
>> to ap_authn_cache_store?
>> Hence wrapping the ->retrieve in a apr_global_mutex_trylock /
>> apr_global_mutex_unlock.
>
> Yes, that is definitely necessary to use the socache API (and it's
> documented that way).
I did create such a patch and asked the reporter to confirm.
>
> I think the use of ctx->pool in mod_socache_dbm can be safely switched
> to the passed-in pool (r->pool in this case) in the
> store/retrieve/remove/status entry points too.
Do you see any reason why we constantly need to get the driver via
apr_dbm_get_driver in case of a later apr-util
and constantly need to apr_dbm_open(2) / apr_dbm_close the DB? Can't we just
open that once in the beginning
and keep it open thru the lifecycle of the server? Or can only one thread have
the DB open due to possible races /
locking issues?
Regards
Rüdiger
