Don't clear the whole error stack when loading engines

Loading the various built-in engines was unconditionally clearing the
whole error stack. During config file processing processing a .include
directive which fails results in errors being added to the stack - but
we carry on anyway. These errors were then later being removed by the
engine loading code, meaning that problems with the .include directive
never get shown.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13311)

diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c
index e7fb890378986000d883bd7eeb1233d2d5060c47..f287cb28eb55b47ac17911f80d6d6e1fb16bdb7f 100644 (file)
--- a/crypto/conf/conf_mod.c
+++ b/crypto/conf/conf_mod.c
@@ -208,7 +208,6 @@ DEFINE_RUN_ONCE_STATIC(do_load_builtin_modules)
     /* Need to load ENGINEs */
     ENGINE_load_builtin_engines();
 #endif
-    ERR_clear_error();
     return 1;
 }
 

diff --git a/crypto/engine/eng_dyn.c b/crypto/engine/eng_dyn.c
index 01935578c26ad1d068644123a952cf6e48cc6258..3b0d8eb91f770bee0bd2590d2f52ab1b114cb6d3 100644 (file)
--- a/crypto/engine/eng_dyn.c
+++ b/crypto/engine/eng_dyn.c
@@ -257,6 +257,8 @@ void engine_load_dynamic_int(void)
     ENGINE *toadd = engine_dynamic();
     if (!toadd)
         return;
+
+    ERR_set_mark();
     ENGINE_add(toadd);
     /*
      * If the "add" worked, it gets a structural reference. So either way, we
@@ -268,7 +270,7 @@ void engine_load_dynamic_int(void)
      * already added (eg. someone calling ENGINE_load_blah then calling
      * ENGINE_load_builtin_engines() perhaps).
      */
-    ERR_clear_error();
+    ERR_pop_to_mark();
 }
 
 static int dynamic_init(ENGINE *e)

diff --git a/crypto/engine/eng_openssl.c b/crypto/engine/eng_openssl.c
index 2374af8ae9ee34f3beb7e6377d491d340e040b49..a51ccf129fc97c9f476f4ca8561a2ad4de9b995c 100644 (file)
--- a/crypto/engine/eng_openssl.c
+++ b/crypto/engine/eng_openssl.c
@@ -152,13 +152,20 @@ void engine_load_openssl_int(void)
     ENGINE *toadd = engine_openssl();
     if (!toadd)
         return;
+
+    ERR_set_mark();
     ENGINE_add(toadd);
     /*
      * If the "add" worked, it gets a structural reference. So either way, we
      * release our just-created reference.
      */
     ENGINE_free(toadd);
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
 }
 
 /*

diff --git a/crypto/engine/eng_rdrand.c b/crypto/engine/eng_rdrand.c
index 39e4055a90c60b12af2e86031b3e8b8227904acb..f46a5145974e62767d051d57c1b27133c191b01a 100644 (file)
--- a/crypto/engine/eng_rdrand.c
+++ b/crypto/engine/eng_rdrand.c
@@ -87,9 +87,19 @@ void engine_load_rdrand_int(void)
         ENGINE *toadd = ENGINE_rdrand();
         if (!toadd)
             return;
+        ERR_set_mark();
         ENGINE_add(toadd);
+        /*
+        * If the "add" worked, it gets a structural reference. So either way, we
+        * release our just-created reference.
+        */
         ENGINE_free(toadd);
-        ERR_clear_error();
+        /*
+        * If the "add" didn't work, it was probably a conflict because it was
+        * already added (eg. someone calling ENGINE_load_blah then calling
+        * ENGINE_load_builtin_engines() perhaps).
+        */
+        ERR_pop_to_mark();
     }
 }
 #else

diff --git a/engines/e_afalg.c b/engines/e_afalg.c
index 24a1aa900c52dee0c47a6848e4d900a6af320077..9480d7c24b005d13849d661fd801091f13ab5beb 100644 (file)
--- a/engines/e_afalg.c
+++ b/engines/e_afalg.c
@@ -851,9 +851,19 @@ void engine_load_afalg_int(void)
     toadd = engine_afalg();
     if (toadd == NULL)
         return;
+    ERR_set_mark();
     ENGINE_add(toadd);
+    /*
+     * If the "add" worked, it gets a structural reference. So either way, we
+     * release our just-created reference.
+     */
     ENGINE_free(toadd);
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
 }
 # endif
 

diff --git a/engines/e_capi.c b/engines/e_capi.c
index 8e5693d25e0a60eed8d31fbacc281050d80b7b53..dd66518d3f5614c690e7ba885a0d0b8bce9b5443 100644 (file)
--- a/engines/e_capi.c
+++ b/engines/e_capi.c
@@ -600,9 +600,19 @@ void engine_load_capi_int(void)
     ENGINE *toadd = engine_capi();
     if (!toadd)
         return;
+    ERR_set_mark();
     ENGINE_add(toadd);
+    /*
+     * If the "add" worked, it gets a structural reference. So either way, we
+     * release our just-created reference.
+     */
     ENGINE_free(toadd);
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
 }
 # endif
 

diff --git a/engines/e_dasync.c b/engines/e_dasync.c
index b817b2ba5f5d2bb66129062e9ea531f5bbaae2ab..4eb50d055cfd8ba74e6b26f2d9e6df9c7c921ae6 100644 (file)
--- a/engines/e_dasync.c
+++ b/engines/e_dasync.c
@@ -348,9 +348,19 @@ void engine_load_dasync_int(void)
     ENGINE *toadd = engine_dasync();
     if (!toadd)
         return;
+    ERR_set_mark();
     ENGINE_add(toadd);
+    /*
+     * If the "add" worked, it gets a structural reference. So either way, we
+     * release our just-created reference.
+     */
     ENGINE_free(toadd);
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
 }
 
 static int dasync_init(ENGINE *e)

diff --git a/engines/e_devcrypto.c b/engines/e_devcrypto.c
index 729bb1fe955af0fa694a030f26dc089cee48ed4f..85815e2e5aeacbbf206ec5da3116d87d53c6ed8e 100644 (file)
--- a/engines/e_devcrypto.c
+++ b/engines/e_devcrypto.c
@@ -1287,9 +1287,20 @@ void engine_load_devcrypto_int(void)
         return;
     }
 
+    ERR_set_mark();
     ENGINE_add(e);
+    /*
+     * If the "add" worked, it gets a structural reference. So either way, we
+     * release our just-created reference.
+     */
     ENGINE_free(e);          /* Loose our local reference */
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
+}
 }
 
 #else

diff --git a/engines/e_padlock.c b/engines/e_padlock.c
index 713a79a36812de01c842a0d56c7be20f1809cde7..572ff9093504a3c92461f3ebeab873c170a7b3e1 100644 (file)
--- a/engines/e_padlock.c
+++ b/engines/e_padlock.c
@@ -49,9 +49,19 @@ void engine_load_padlock_int(void)
     ENGINE *toadd = ENGINE_padlock();
     if (!toadd)
         return;
+    ERR_set_mark();
     ENGINE_add(toadd);
+    /*
+     * If the "add" worked, it gets a structural reference. So either way, we
+     * release our just-created reference.
+     */
     ENGINE_free(toadd);
-    ERR_clear_error();
+    /*
+     * If the "add" didn't work, it was probably a conflict because it was
+     * already added (eg. someone calling ENGINE_load_blah then calling
+     * ENGINE_load_builtin_engines() perhaps).
+     */
+    ERR_pop_to_mark();
 #  endif
 }