summaryrefslogtreecommitdiffstats
path: root/crypto/engine
diff options
context:
space:
mode:
Diffstat (limited to 'crypto/engine')
-rw-r--r--crypto/engine/engine.h14
-rw-r--r--crypto/engine/engine_all.c6
-rw-r--r--crypto/engine/engine_int.h23
-rw-r--r--crypto/engine/engine_lib.c103
-rw-r--r--crypto/engine/engine_list.c74
5 files changed, 154 insertions, 66 deletions
diff --git a/crypto/engine/engine.h b/crypto/engine/engine.h
index 258ec6ec43..3cb52254ff 100644
--- a/crypto/engine/engine.h
+++ b/crypto/engine/engine.h
@@ -363,8 +363,12 @@ int ENGINE_cpy(ENGINE *dest, const ENGINE *src);
int ENGINE_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
int ENGINE_set_ex_data(ENGINE *e, int idx, void *arg);
-/* Cleans the internal engine structure. This should only be used when the
- * application is about to exit. */
+/* Cleans the internal engine list. This should only be used when the
+ * application is about to exit or restart operation (the next operation
+ * requiring the ENGINE list will re-initialise it with defaults). NB: Dynamic
+ * ENGINEs will only truly unload (including any allocated data or loaded
+ * shared-libraries) if all remaining references are released too - so keys,
+ * certificates, etc all need to be released for an in-use ENGINE to unload. */
void ENGINE_cleanup(void);
/* These return values from within the ENGINE structure. These can be useful
@@ -445,6 +449,12 @@ int ENGINE_set_default_BN_mod_exp_crt(ENGINE *e);
* ENGINE_METHOD_*** defines above. */
int ENGINE_set_default(ENGINE *e, unsigned int flags);
+/* This function resets all the internal "default" ENGINEs (there's one for each
+ * of the various algorithms) to NULL, releasing any references as appropriate.
+ * This function is called as part of the ENGINE_cleanup() function, so there's
+ * no need to call both (although no harm is done). */
+int ENGINE_clear_defaults(void);
+
/* Obligatory error function. */
void ERR_load_ENGINE_strings(void);
diff --git a/crypto/engine/engine_all.c b/crypto/engine/engine_all.c
index 43da60483b..4d0244f351 100644
--- a/crypto/engine/engine_all.c
+++ b/crypto/engine/engine_all.c
@@ -62,12 +62,14 @@
static int engine_add(ENGINE *e)
{
+ int toret = 1;
if (!ENGINE_by_id(ENGINE_get_id(e)))
{
(void)ERR_get_error();
- return ENGINE_add(e);
+ toret = ENGINE_add(e);
}
- return 1;
+ ENGINE_free(e);
+ return toret;
}
void ENGINE_load_cswift(void)
diff --git a/crypto/engine/engine_int.h b/crypto/engine/engine_int.h
index d44f648559..54cfe47af7 100644
--- a/crypto/engine/engine_int.h
+++ b/crypto/engine/engine_int.h
@@ -66,6 +66,29 @@
extern "C" {
#endif
+#define ENGINE_REF_COUNT_DEBUG
+
+/* If we compile with this symbol defined, then both reference counts in the
+ * ENGINE structure will be monitored with a line of output on stderr for each
+ * change. This prints the engine's pointer address (truncated to unsigned int),
+ * "struct" or "funct" to indicate the reference type, the before and after
+ * reference count, and the file:line-number pair. The "engine_ref_debug"
+ * statements must come *after* the change. */
+#ifdef ENGINE_REF_COUNT_DEBUG
+
+#define engine_ref_debug(e, isfunct, diff) \
+ fprintf(stderr, "blargle: %08x %s from %d to %d (%s:%d)\n", \
+ (unsigned int)(e), (isfunct ? "funct" : "struct"), \
+ ((isfunct) ? ((e)->funct_ref - (diff)) : ((e)->struct_ref - (diff))), \
+ ((isfunct) ? (e)->funct_ref : (e)->struct_ref), \
+ (__FILE__), (__LINE__));
+
+#else
+
+#define engine_ref_debug(e, isfunct, diff)
+
+#endif
+
/* NB: Bitwise OR-able values for the "flags" variable in ENGINE are now exposed
* in engine.h. */
diff --git a/crypto/engine/engine_lib.c b/crypto/engine/engine_lib.c
index 5925a0bc65..90eb5cd6d5 100644
--- a/crypto/engine/engine_lib.c
+++ b/crypto/engine/engine_lib.c
@@ -103,6 +103,8 @@ static void engine_def_check_util(ENGINE **def, ENGINE *val)
*def = val;
val->struct_ref++;
val->funct_ref++;
+ engine_ref_debug(val, 0, 1)
+ engine_ref_debug(val, 1, 1)
}
/* In a slight break with convention - this static function must be
@@ -176,6 +178,8 @@ int ENGINE_init(ENGINE *e)
* structural reference. */
e->struct_ref++;
e->funct_ref++;
+ engine_ref_debug(e, 0, 1)
+ engine_ref_debug(e, 1, 1)
}
CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
return to_return;
@@ -192,43 +196,38 @@ int ENGINE_finish(ENGINE *e)
return 0;
}
CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
- if((e->funct_ref == 1) && e->finish)
-#if 0
- /* This is the last functional reference and the engine
- * requires cleanup so we do it now. */
- to_return = e->finish();
- if(to_return)
- {
- /* Cleanup the functional reference which is also a
- * structural reference. */
- e->struct_ref--;
- e->funct_ref--;
- }
- CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
-#else
- /* I'm going to deliberately do a convoluted version of this
- * piece of code because we don't want "finish" functions
- * being called inside a locked block of code, if at all
- * possible. I'd rather have this call take an extra couple
- * of ticks than have throughput serialised on a externally-
- * provided callback function that may conceivably never come
- * back. :-( */
+ /* Reduce the functional reference count here so if it's the terminating
+ * case, we can release the lock safely and call the finish() handler
+ * without risk of a race. We get a race if we leave the count until
+ * after and something else is calling "finish" at the same time -
+ * there's a chance that both threads will together take the count from
+ * 2 to 0 without either calling finish(). */
+ e->funct_ref--;
+ engine_ref_debug(e, 1, -1)
+ if((e->funct_ref == 0) && e->finish)
{
CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
- /* CODE ALERT: This *IS* supposed to be "=" and NOT "==" :-) */
- if((to_return = e->finish(e)))
+ if(!(to_return = e->finish(e)))
{
- CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
- /* Cleanup the functional reference which is also a
- * structural reference. */
- e->struct_ref--;
- e->funct_ref--;
- CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+ ENGINEerr(ENGINE_F_ENGINE_FINISH,ENGINE_R_FINISH_FAILED);
+ return 0;
}
}
else
CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+#ifdef REF_CHECK
+ if(e->funct_ref < 0)
+ {
+ fprintf(stderr,"ENGINE_finish, bad functional reference count\n");
+ abort();
+ }
#endif
+ /* Release the structural reference too */
+ if(!ENGINE_free(e))
+ {
+ ENGINEerr(ENGINE_F_ENGINE_FINISH,ENGINE_R_FINISH_FAILED);
+ return 0;
+ }
return to_return;
}
@@ -620,8 +619,8 @@ static ENGINE *engine_get_default_type(ENGINE_TYPE t)
ret = engine_def_bn_mod_exp; break;
case ENGINE_TYPE_BN_MOD_EXP_CRT:
ret = engine_def_bn_mod_exp_crt; break;
- default:
- break;
+ default:
+ break;
}
/* Unforunately we can't do this work outside the lock with a
* call to ENGINE_init() because that would leave a race
@@ -630,6 +629,8 @@ static ENGINE *engine_get_default_type(ENGINE_TYPE t)
{
ret->struct_ref++;
ret->funct_ref++;
+ engine_ref_debug(ret, 0, 1)
+ engine_ref_debug(ret, 1, 1)
}
CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
return ret;
@@ -675,12 +676,6 @@ static int engine_set_default_type(ENGINE_TYPE t, ENGINE *e)
{
ENGINE *old = NULL;
- if(e == NULL)
- {
- ENGINEerr(ENGINE_F_ENGINE_SET_DEFAULT_TYPE,
- ERR_R_PASSED_NULL_PARAMETER);
- return 0;
- }
/* engine_def_check is lean and mean and won't replace any
* prior default engines ... so we must ensure that it is always
* the first function to get to touch the default values. */
@@ -688,7 +683,7 @@ static int engine_set_default_type(ENGINE_TYPE t, ENGINE *e)
/* Attempt to get a functional reference (we need one anyway, but
* also, 'e' may be just a structural reference being passed in so
* this call may actually be the first). */
- if(!ENGINE_init(e))
+ if(e && !ENGINE_init(e))
{
ENGINEerr(ENGINE_F_ENGINE_SET_DEFAULT_TYPE,
ENGINE_R_INIT_FAILED);
@@ -721,8 +716,8 @@ static int engine_set_default_type(ENGINE_TYPE t, ENGINE *e)
case ENGINE_TYPE_BN_MOD_EXP_CRT:
old = engine_def_bn_mod_exp_crt;
engine_def_bn_mod_exp_crt = e; break;
- default:
- break;
+ default:
+ break;
}
CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
/* If we've replaced a previous value, then we need to remove the
@@ -801,3 +796,31 @@ int ENGINE_set_default(ENGINE *e, unsigned int flags)
return 1;
}
+int ENGINE_clear_defaults(void)
+ {
+ /* If the defaults haven't even been set yet, don't bother. Any kind of
+ * "cleanup" has a kind of implicit race-condition if another thread is
+ * trying to keep going, so we don't address that with locking. The
+ * first ENGINE_set_default_*** call will actually *create* a standard
+ * set of default ENGINEs (including init() and functional reference
+ * counts aplenty) before the rest of this function undoes them all. So
+ * save some hassle ... */
+ if(!engine_def_flag)
+ return 1;
+ if((0 == 1) ||
+#ifndef OPENSSL_NO_RSA
+ !ENGINE_set_default_RSA(NULL) ||
+#endif
+#ifndef OPENSSL_NO_DSA
+ !ENGINE_set_default_DSA(NULL) ||
+#endif
+#ifndef OPENSSL_NO_DH
+ !ENGINE_set_default_DH(NULL) ||
+#endif
+ !ENGINE_set_default_RAND(NULL) ||
+ !ENGINE_set_default_BN_mod_exp(NULL) ||
+ !ENGINE_set_default_BN_mod_exp_crt(NULL))
+ return 0;
+ return 1;
+ }
+
diff --git a/crypto/engine/engine_list.c b/crypto/engine/engine_list.c
index 0f6e9bb242..b41e6e5354 100644
--- a/crypto/engine/engine_list.c
+++ b/crypto/engine/engine_list.c
@@ -139,6 +139,7 @@ static int engine_list_add(ENGINE *e)
/* Having the engine in the list assumes a structural
* reference. */
e->struct_ref++;
+ engine_ref_debug(e, 0, 1)
/* However it came to be, e is the last item in the list. */
engine_list_tail = e;
e->next = NULL;
@@ -177,6 +178,7 @@ static int engine_list_remove(ENGINE *e)
engine_list_tail = e->prev;
/* remove our structural reference. */
e->struct_ref--;
+ engine_ref_debug(e, 0, -1)
return 1;
}
@@ -187,13 +189,24 @@ static int engine_list_remove(ENGINE *e)
* as it will generate errors itself. */
static int engine_internal_check(void)
{
+ int toret = 1;
+ ENGINE *def_engine;
if(engine_list_flag)
return 1;
/* This is our first time up, we need to populate the list
* with our statically compiled-in engines. */
- if(!engine_list_add(ENGINE_openssl()))
- return 0;
- engine_list_flag = 1;
+ def_engine = ENGINE_openssl();
+ if(!engine_list_add(def_engine))
+ toret = 0;
+ else
+ engine_list_flag = 1;
+#if 0
+ ENGINE_free(def_engine);
+#else
+ /* We can't ENGINE_free() because the lock's already held */
+ def_engine->struct_ref--;
+ engine_ref_debug(def_engine, 0, -1)
+#endif
return 1;
}
@@ -207,7 +220,10 @@ ENGINE *ENGINE_get_first(void)
{
ret = engine_list_head;
if(ret)
+ {
ret->struct_ref++;
+ engine_ref_debug(ret, 0, 1)
+ }
}
CRYPTO_r_unlock(CRYPTO_LOCK_ENGINE);
return ret;
@@ -221,7 +237,10 @@ ENGINE *ENGINE_get_last(void)
{
ret = engine_list_tail;
if(ret)
+ {
ret->struct_ref++;
+ engine_ref_debug(ret, 0, 1)
+ }
}
CRYPTO_r_unlock(CRYPTO_LOCK_ENGINE);
return ret;
@@ -240,8 +259,11 @@ ENGINE *ENGINE_get_next(ENGINE *e)
CRYPTO_r_lock(CRYPTO_LOCK_ENGINE);
ret = e->next;
if(ret)
+ {
/* Return a valid structural refernce to the next ENGINE */
ret->struct_ref++;
+ engine_ref_debug(ret, 0, 1)
+ }
CRYPTO_r_unlock(CRYPTO_LOCK_ENGINE);
/* Release the structural reference to the previous ENGINE */
ENGINE_free(e);
@@ -259,8 +281,11 @@ ENGINE *ENGINE_get_prev(ENGINE *e)
CRYPTO_r_lock(CRYPTO_LOCK_ENGINE);
ret = e->prev;
if(ret)
+ {
/* Return a valid structural reference to the next ENGINE */
ret->struct_ref++;
+ engine_ref_debug(ret, 0, 1)
+ }
CRYPTO_r_unlock(CRYPTO_LOCK_ENGINE);
/* Release the structural reference to the previous ENGINE */
ENGINE_free(e);
@@ -349,7 +374,10 @@ ENGINE *ENGINE_by_id(const char *id)
}
}
else
+ {
iterator->struct_ref++;
+ engine_ref_debug(iterator, 0, 1)
+ }
}
}
CRYPTO_r_unlock(CRYPTO_LOCK_ENGINE);
@@ -371,6 +399,7 @@ ENGINE *ENGINE_new(void)
}
memset(ret, 0, sizeof(ENGINE));
ret->struct_ref = 1;
+ engine_ref_debug(ret, 0, 1)
CRYPTO_new_ex_data(engine_ex_data_stack, ret, &ret->ex_data);
return ret;
}
@@ -386,14 +415,12 @@ int ENGINE_free(ENGINE *e)
return 0;
}
i = CRYPTO_add(&e->struct_ref,-1,CRYPTO_LOCK_ENGINE);
-#ifdef REF_PRINT
- REF_PRINT("ENGINE",e);
-#endif
+ engine_ref_debug(e, 0, -1)
if (i > 0) return 1;
#ifdef REF_CHECK
if (i < 0)
{
- fprintf(stderr,"ENGINE_free, bad reference count\n");
+ fprintf(stderr,"ENGINE_free, bad structural reference count\n");
abort();
}
#endif
@@ -422,18 +449,21 @@ void *ENGINE_get_ex_data(const ENGINE *e, int idx)
}
void ENGINE_cleanup(void)
- {
- ENGINE *iterator = engine_list_head;
-
- while(iterator != NULL)
- {
- ENGINE_remove(iterator);
- ENGINE_free(iterator);
- iterator = engine_list_head;
- }
- engine_list_flag = 0;
- return;
- }
+ {
+ ENGINE *iterator = engine_list_head;
+
+ while(iterator != NULL)
+ {
+ ENGINE_remove(iterator);
+ iterator = engine_list_head;
+ }
+ engine_list_flag = 0;
+ /* Also unset any "default" ENGINEs that may have been set up (a default
+ * constitutes a functional reference on an ENGINE and there's one for
+ * each algorithm). */
+ ENGINE_clear_defaults();
+ return;
+ }
int ENGINE_set_id(ENGINE *e, const char *id)
{
@@ -465,7 +495,7 @@ int ENGINE_set_RSA(ENGINE *e, const RSA_METHOD *rsa_meth)
e->rsa_meth = rsa_meth;
return 1;
#else
- return 0;
+ return 0;
#endif
}
@@ -475,7 +505,7 @@ int ENGINE_set_DSA(ENGINE *e, const DSA_METHOD *dsa_meth)
e->dsa_meth = dsa_meth;
return 1;
#else
- return 0;
+ return 0;
#endif
}
@@ -485,7 +515,7 @@ int ENGINE_set_DH(ENGINE *e, const DH_METHOD *dh_meth)
e->dh_meth = dh_meth;
return 1;
#else
- return 0;
+ return 0;
#endif
}