On Mon, Sep 12, 2016 at 10:31 AM, Ewald Dieterich <ew...@mailbox.org> wrote:
> On 06/13/2016 09:38 AM, Ewald Dieterich wrote:
>
> Looks like the problem is this:
Thanks for invertigating!
>
> * In session_crypto_init() a crypto context is created from a global pool
> (server->pconf).
> * In encrypt_string() and decrypt_string() a key is created from the context
> via apr_crypto_passphrase() using the global pool for allocating memory for
> the key.
True, this is because we give apr_crypto_passphrase() a NULL key.
> * Multiple threads might access the global pool at the same time.
Correct, hence the data corruption (possibly crash).
>
> I changed mod_session_crypto to use the request pool instead and it seems
> that this fixed my problem.
>
> I think this also fixes a memory consumption problem: Keys are only created,
> but never explicitly destroyed (or reused). So for every request memory is
> allocated from the global pool, but this memory is never freed during the
> lifetime of mod_session_crypto. Using the request pool fixes this problem
> because it is destroyed when the request is done.
>
> See the attached patch session-crypto.patch that I created for 2.4.20.
Thanks for the patch, possibly a simpler way to fix it would be:
Index: modules/session/mod_session_crypto.c
===================================================================
--- modules/session/mod_session_crypto.c (revision 1760149)
+++ modules/session/mod_session_crypto.c (working copy)
@@ -246,6 +246,8 @@ static apr_status_t decrypt_string(request_rec * r
return res;
}
+ key = apr_pcalloc(r->pool, sizeof *key);
+
/* try each passphrase in turn */
for (; i < dconf->passphrases->nelts; i++) {
const char *passphrase = APR_ARRAY_IDX(dconf->passphrases, i, char *);
_
so that crypto_passphrase() will use the given key instead of
allocating a new one.
Regards,
Yann.
