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 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. > I guess we can still use apr_global_mutex_trylock and return > AUTH_USER_NOT_FOUND in case > we do not get the lock? Seems reasonable to me, yup.
