apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: bugs in apr_crypto_get_driver()
Date Sun, 01 Apr 2012 15:25:32 GMT
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
--


Mime
View raw message