On 01 Apr 2012, at 12:07 AM, Greg Stein wrote:

> I've identified two problems in apr_crypto_get_driver(), and need
> somebody to review my thinking and make the appropriate corrections.
> 
> First up is this block:
> 
>    rv = apu_dso_load(&dso, &symbol, modname, symname, pool);
>    if (rv != APR_SUCCESS) { /* APR_EDSOOPEN or APR_ESYMNOTFOUND? */
>        if (rv == APR_EINIT) { /* previously loaded?!? */
>            name = apr_pstrdup(pool, name);
>            apr_hash_set(drivers, name, APR_HASH_KEY_STRING, *driver);
>            rv = APR_SUCCESS;
> 
> That apr_hash_set() will store a NULL, since the only way here is when
> *driver == NULL. Further, it certainly shouldn't set rv to APR_SUCCESS
> since nothing got loaded and returned in *driver.

Looks like this issue was fixed in apr_dbd, but not in apr_crypto. I've ported 
the fix across.

> The second problem is the series of DRIVER_LOAD() calls at the bottom
> of the function. None of these appear to set *driver to the
> appropriate value. (and beware macro arg name shadowing)

I think we were getting away with this because httpd initialises twice. The 
first time it fails, but it doesn't matter because we initialise a second time, 
at which point the driver is already initialised and correctly returned from 
the driver list, and so works. It's now fixed and the tests pass.

I've backported both fixes to apr-util v1.4, and will reroll the v1.4.2 
release. Can you verify that these changes seem sensible?

Regards,
Graham
--

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to