You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2023/03/17 15:17:01 UTC

svn commit: r1908453 - /apr/apr/trunk/crypto/apr_crypto_openssl.c

Author: ylavic
Date: Fri Mar 17 15:17:01 2023
New Revision: 1908453

URL: http://svn.apache.org/viewvc?rev=1908453&view=rev
Log:
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().


Modified:
    apr/apr/trunk/crypto/apr_crypto_openssl.c

Modified: apr/apr/trunk/crypto/apr_crypto_openssl.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/crypto/apr_crypto_openssl.c?rev=1908453&r1=1908452&r2=1908453&view=diff
==============================================================================
--- apr/apr/trunk/crypto/apr_crypto_openssl.c (original)
+++ apr/apr/trunk/crypto/apr_crypto_openssl.c Fri Mar 17 15:17:01 2023
@@ -301,10 +301,12 @@ static apr_status_t crypto_key_cleanup(a
 {
     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_cryp
         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_cryp
     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_cryp
         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_cryp
             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_cryp
 
     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_cryp
 
     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_crypt
         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
     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
 {
     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(a
 {
     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(a
     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: {