Fix and simplify error handling in (RSA/EC_kmeth)_new_method()

Inspired from PR #873.
Nearly same as 2bbf0ba.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>

diff --git a/crypto/ec/ec_kmeth.c b/crypto/ec/ec_kmeth.c
index fead014ebc529e8a19664cb0a811002181bd3bab..eb469ba3adfe8ff1da4e6d2e8a81fef61e6c40d3 100644 (file)
--- a/crypto/ec/ec_kmeth.c
+++ b/crypto/ec/ec_kmeth.c
@@ -78,15 +78,11 @@ EC_KEY *EC_KEY_new_method(ENGINE *engine)
         ECerr(EC_F_EC_KEY_NEW_METHOD, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_EC_KEY, ret, &ret->ex_data)) {
-        OPENSSL_free(ret);
-        return NULL;
-    }
 
+    ret->references = 1;
     ret->lock = CRYPTO_THREAD_lock_new();
     if (ret->lock == NULL) {
         ECerr(EC_F_EC_KEY_NEW_METHOD, ERR_R_MALLOC_FAILURE);
-        CRYPTO_free_ex_data(CRYPTO_EX_INDEX_EC_KEY, ret, &ret->ex_data);
         OPENSSL_free(ret);
         return NULL;
     }
@@ -96,10 +92,7 @@ EC_KEY *EC_KEY_new_method(ENGINE *engine)
     if (engine != NULL) {
         if (!ENGINE_init(engine)) {
             ECerr(EC_F_EC_KEY_NEW_METHOD, ERR_R_ENGINE_LIB);
-            CRYPTO_free_ex_data(CRYPTO_EX_INDEX_EC_KEY, ret, &ret->ex_data);
-            CRYPTO_THREAD_lock_free(ret->lock);
-            OPENSSL_free(ret);
-            return NULL;
+            goto err;
         }
         ret->engine = engine;
     } else
@@ -108,25 +101,27 @@ EC_KEY *EC_KEY_new_method(ENGINE *engine)
         ret->meth = ENGINE_get_EC(ret->engine);
         if (ret->meth == NULL) {
             ECerr(EC_F_EC_KEY_NEW_METHOD, ERR_R_ENGINE_LIB);
-            ENGINE_finish(ret->engine);
-            CRYPTO_free_ex_data(CRYPTO_EX_INDEX_EC_KEY, ret, &ret->ex_data);
-            CRYPTO_THREAD_lock_free(ret->lock);
-            OPENSSL_free(ret);
-            return NULL;
+            goto err;
         }
     }
 #endif
 
     ret->version = 1;
     ret->conv_form = POINT_CONVERSION_UNCOMPRESSED;
-    ret->references = 1;
+
+    if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_EC_KEY, ret, &ret->ex_data)) {
+        goto err;
+    }
 
     if (ret->meth->init != NULL && ret->meth->init(ret) == 0) {
         ECerr(EC_F_EC_KEY_NEW_METHOD, ERR_R_INIT_FAIL);
-        EC_KEY_free(ret);
-        return NULL;
+        goto err;
     }
     return ret;
+
+err:
+    EC_KEY_free(ret);
+    return NULL;
 }
 
 int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,

diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c
index 0ccb3ce02fbcb2cfeff79cd061c8a12041a22a08..4f93cbcc4301c052016ffcc47fa3089014cce38c 100644 (file)
--- a/crypto/rsa/rsa_lib.c
+++ b/crypto/rsa/rsa_lib.c
@@ -70,21 +70,28 @@ int RSA_set_method(RSA *rsa, const RSA_METHOD *meth)
 
 RSA *RSA_new_method(ENGINE *engine)
 {
-    RSA *ret;
+    RSA *ret = OPENSSL_zalloc(sizeof(*ret));
 
-    ret = OPENSSL_zalloc(sizeof(*ret));
     if (ret == NULL) {
         RSAerr(RSA_F_RSA_NEW_METHOD, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
 
+    ret->references = 1;
+    ret->lock = CRYPTO_THREAD_lock_new();
+    if (ret->lock == NULL) {
+        RSAerr(RSA_F_RSA_NEW_METHOD, ERR_R_MALLOC_FAILURE);
+        OPENSSL_free(ret);
+        return NULL;
+    }
+
     ret->meth = RSA_get_default_method();
 #ifndef OPENSSL_NO_ENGINE
+    ret->flags = ret->meth->flags & ~RSA_FLAG_NON_FIPS_ALLOW;
     if (engine) {
         if (!ENGINE_init(engine)) {
             RSAerr(RSA_F_RSA_NEW_METHOD, ERR_R_ENGINE_LIB);
-            OPENSSL_free(ret);
-            return NULL;
+            goto err;
         }
         ret->engine = engine;
     } else
@@ -93,39 +100,26 @@ RSA *RSA_new_method(ENGINE *engine)
         ret->meth = ENGINE_get_RSA(ret->engine);
         if (ret->meth == NULL) {
             RSAerr(RSA_F_RSA_NEW_METHOD, ERR_R_ENGINE_LIB);
-            ENGINE_finish(ret->engine);
-            OPENSSL_free(ret);
-            return NULL;
+            goto err;
         }
     }
 #endif
 
-    ret->references = 1;
     ret->flags = ret->meth->flags & ~RSA_FLAG_NON_FIPS_ALLOW;
     if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_RSA, ret, &ret->ex_data)) {
-#ifndef OPENSSL_NO_ENGINE
-        ENGINE_finish(ret->engine);
-#endif
-        OPENSSL_free(ret);
-        return NULL;
-    }
-
-    ret->lock = CRYPTO_THREAD_lock_new();
-    if (ret->lock == NULL) {
-#ifndef OPENSSL_NO_ENGINE
-        ENGINE_finish(ret->engine);
-#endif
-        CRYPTO_free_ex_data(CRYPTO_EX_INDEX_RSA, ret, &ret->ex_data);
-        OPENSSL_free(ret);
-        return NULL;
+        goto err;
     }
 
     if ((ret->meth->init != NULL) && !ret->meth->init(ret)) {
-        RSA_free(ret);
-        ret = NULL;
+        RSAerr(RSA_F_RSA_NEW_METHOD, ERR_R_INIT_FAIL);
+        goto err;
     }
 
     return ret;
+
+err:
+    RSA_free(ret);
+    return NULL;
 }
 
 void RSA_free(RSA *r)