From 24ba1072a3e833f54fd10df3aa6da64331e0244e Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Fri, 17 Mar 2023 15:17:01 +0000 Subject: crypto: Follow up to r1908433: streamline cleanups. The crypto_*_init() functions can be called multiple times with the returned structure already allocated/initialized, or not. So rather than cleaning up after each operation (which was removed in r1908433 but is now leaky), let's instead either allocate+register or cleanup existing structure in _init(). git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1908453 13f79535-47bb-0310-9956-ffa450edef68 --- crypto/apr_crypto_openssl.c | 108 +++++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 46 deletions(-) diff --git a/crypto/apr_crypto_openssl.c b/crypto/apr_crypto_openssl.c index a13e7fc43..ee355bcd4 100644 --- a/crypto/apr_crypto_openssl.c +++ b/crypto/apr_crypto_openssl.c @@ -301,10 +301,12 @@ static apr_status_t crypto_key_cleanup(apr_crypto_key_t *key) { if (key->pkey) { EVP_PKEY_free(key->pkey); + key->pkey = NULL; } #if !APR_USE_OPENSSL_PRE_3_0_API if (key->mac) { EVP_MAC_free(key->mac); + key->mac = NULL; } #endif @@ -417,10 +419,9 @@ static apr_status_t crypto_make(apr_crypto_t **ff, apr_pool_t *pool) { apr_crypto_t *f; - apr_crypto_config_t *config = NULL; - + apr_crypto_config_t *config; const char *engine = NULL; - + apr_status_t status = APR_SUCCESS; struct { const char *field; const char *value; @@ -434,17 +435,8 @@ static apr_status_t crypto_make(apr_crypto_t **ff, char **elts = NULL; char *elt; int i = 0, j; - apr_status_t status; - - f = apr_pcalloc(pool, sizeof(apr_crypto_t)); - if (!f) { - return APR_ENOMEM; - } - config = f->config = apr_pcalloc(pool, sizeof(apr_crypto_config_t)); - if (!config) { - return APR_ENOMEM; - } + *ff = NULL; if (params) { if (APR_SUCCESS != (status = apr_tokenize_to_argv(params, &elts, pool))) { @@ -478,6 +470,17 @@ static apr_status_t crypto_make(apr_crypto_t **ff, engine = fields[0].value; } + f = apr_pcalloc(pool, sizeof(apr_crypto_t)); + if (!f) { + return APR_ENOMEM; + } + f->config = config = apr_pcalloc(pool, sizeof(apr_crypto_config_t)); + if (!config) { + return APR_ENOMEM; + } + f->pool = pool; + f->provider = provider; + /* The default/builtin "openssl" engine is the same as NULL though with * openssl-3+ it's called something else, keep NULL for that name. */ @@ -488,27 +491,24 @@ static apr_status_t crypto_make(apr_crypto_t **ff, return APR_ENOENGINE; } if (!ENGINE_init(config->engine)) { - ENGINE_free(config->engine); - config->engine = NULL; - return APR_EINITENGINE; + status = APR_EINITENGINE; + goto cleanup; } #else return APR_ENOTIMPL; #endif } - *ff = f; - f->pool = pool; - f->provider = provider; - f->result = apr_pcalloc(pool, sizeof(apu_err_t)); if (!f->result) { - return APR_ENOMEM; + status = APR_ENOMEM; + goto cleanup; } f->digests = apr_hash_make(pool); if (!f->digests) { - return APR_ENOMEM; + status = APR_ENOMEM; + goto cleanup; } apr_hash_set(f->digests, "md5", APR_HASH_KEY_STRING, &(key_digests[i = 0])); apr_hash_set(f->digests, "sha1", APR_HASH_KEY_STRING, &(key_digests[++i])); @@ -519,7 +519,8 @@ static apr_status_t crypto_make(apr_crypto_t **ff, f->types = apr_hash_make(pool); if (!f->types) { - return APR_ENOMEM; + status = APR_ENOMEM; + goto cleanup; } apr_hash_set(f->types, "3des192", APR_HASH_KEY_STRING, &(key_types[i = 0])); apr_hash_set(f->types, "aes128", APR_HASH_KEY_STRING, &(key_types[++i])); @@ -528,21 +529,26 @@ static apr_status_t crypto_make(apr_crypto_t **ff, f->modes = apr_hash_make(pool); if (!f->modes) { - return APR_ENOMEM; + status = APR_ENOMEM; + goto cleanup; } apr_hash_set(f->modes, "ecb", APR_HASH_KEY_STRING, &(key_modes[i = 0])); apr_hash_set(f->modes, "cbc", APR_HASH_KEY_STRING, &(key_modes[++i])); f->digests = apr_hash_make(pool); if (!f->digests) { - return APR_ENOMEM; + status = APR_ENOMEM; + goto cleanup; } + *ff = f; apr_pool_cleanup_register(pool, f, crypto_cleanup_helper, - apr_pool_cleanup_null); - + apr_pool_cleanup_null); return APR_SUCCESS; +cleanup: + crypto_cleanup(f); + return status; } /** @@ -687,11 +693,12 @@ static apr_status_t crypto_key(apr_crypto_key_t **k, if (!key) { return APR_ENOMEM; } + apr_pool_cleanup_register(p, key, crypto_key_cleanup_helper, + apr_pool_cleanup_null); + } + else { + crypto_key_cleanup(key); } - - apr_pool_cleanup_register(p, key, crypto_key_cleanup_helper, - apr_pool_cleanup_null); - key->pool = p; key->f = f; key->provider = f->provider; @@ -955,20 +962,23 @@ static apr_status_t crypto_block_encrypt_init(apr_crypto_block_t **ctx, unsigned char *usedIv; apr_crypto_config_t *config = key->f->config; apr_crypto_block_t *block = *ctx; + if (!block) { *ctx = block = apr_pcalloc(p, sizeof(apr_crypto_block_t)); + if (!block) { + return APR_ENOMEM; + } + apr_pool_cleanup_register(p, block, crypto_block_cleanup_helper, + apr_pool_cleanup_null); } - if (!block) { - return APR_ENOMEM; + else { + crypto_block_cleanup(block); } block->f = key->f; block->pool = p; block->provider = key->provider; block->key = key; - apr_pool_cleanup_register(p, block, crypto_block_cleanup_helper, - apr_pool_cleanup_null); - switch (key->rec->ktype) { case APR_CRYPTO_KTYPE_PASSPHRASE: @@ -1172,20 +1182,23 @@ static apr_status_t crypto_block_decrypt_init(apr_crypto_block_t **ctx, { apr_crypto_config_t *config = key->f->config; apr_crypto_block_t *block = *ctx; + if (!block) { *ctx = block = apr_pcalloc(p, sizeof(apr_crypto_block_t)); + if (!block) { + return APR_ENOMEM; + } + apr_pool_cleanup_register(p, block, crypto_block_cleanup_helper, + apr_pool_cleanup_null); } - if (!block) { - return APR_ENOMEM; + else { + crypto_block_cleanup(block); } block->f = key->f; block->pool = p; block->provider = key->provider; block->key = key; - apr_pool_cleanup_register(p, block, crypto_block_cleanup_helper, - apr_pool_cleanup_null); - switch (key->rec->ktype) { case APR_CRYPTO_KTYPE_PASSPHRASE: @@ -1358,11 +1371,17 @@ static apr_status_t crypto_digest_init(apr_crypto_digest_t **d, { apr_crypto_config_t *config = key->f->config; apr_crypto_digest_t *digest = *d; + if (!digest) { *d = digest = apr_pcalloc(p, sizeof(apr_crypto_digest_t)); + if (!digest) { + return APR_ENOMEM; + } + apr_pool_cleanup_register(p, digest, crypto_digest_cleanup_helper, + apr_pool_cleanup_null); } - if (!digest) { - return APR_ENOMEM; + else { + crypto_digest_cleanup(digest); } digest->f = key->f; digest->pool = p; @@ -1370,9 +1389,6 @@ static apr_status_t crypto_digest_init(apr_crypto_digest_t **d, digest->key = key; digest->rec = rec; - apr_pool_cleanup_register(p, digest, crypto_digest_cleanup_helper, - apr_pool_cleanup_null); - switch (key->rec->ktype) { case APR_CRYPTO_KTYPE_HASH: { -- cgit v1.2.1