Locking issues.
diff --git a/CHANGES b/CHANGES
index d198703..5c31f05 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,23 @@
Changes between 0.9.6 and 0.9.7 [xx XXX 2000]
+ *) Add functionality to apps/openssl.c for detecting locking
+ problems: As the program is single-threaded, all we have
+ to do is register a locking callback using an array for
+ storing which locks are currently held by the program.
+
+ Fix a deadlock in CRYPTO_mem_leaks() that was detected in
+ apps/openssl.c.
+ [Bodo Moeller]
+
+ *) Use a lock around the call to CRYPTO_get_ex_new_index() in
+ SSL_get_ex_data_X509_STORE_idx(), which is used in
+ ssl_verify_cert_chain() and thus can be called at any time
+ during TLS/SSL handshakes so that thread-safety is essential.
+ Unfortunately, the ex_data design is not at all suited
+ for multi-threaded use, so it probably should be abolished.
+ [Bodo Moeller]
+
*) Added Broadcom "ubsec" ENGINE to OpenSSL.
[Broadcom, tweaked and integrated by Geoff Thorpe]
diff --git a/apps/openssl.c b/apps/openssl.c
index 4ec9606..6c69a29 100644
--- a/apps/openssl.c
+++ b/apps/openssl.c
@@ -55,6 +55,60 @@
* copied and put under another distribution licence
* [including the GNU Public Licence.]
*/
+/* ====================================================================
+ * Copyright (c) 1998-2000 The OpenSSL Project. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ * software must display the following acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit. (http://www.openssl.org/)"
+ *
+ * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
+ * endorse or promote products derived from this software without
+ * prior written permission. For written permission, please contact
+ * openssl-core@openssl.org.
+ *
+ * 5. Products derived from this software may not be called "OpenSSL"
+ * nor may "OpenSSL" appear in their names without prior written
+ * permission of the OpenSSL Project.
+ *
+ * 6. Redistributions of any form whatsoever must retain the following
+ * acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit (http://www.openssl.org/)"
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This product includes cryptographic software written by Eric Young
+ * (eay@cryptsoft.com). This product includes software written by Tim
+ * Hudson (tjh@cryptsoft.com).
+ *
+ */
+
#include <stdio.h>
#include <string.h>
@@ -92,6 +146,71 @@
BIO *bio_err=NULL;
#endif
+
+static void lock_dbg_cb(int mode, int type, const char *file, int line)
+ {
+ static int modes[CRYPTO_NUM_LOCKS]; /* = {0, 0, ... } */
+ const char *errstr = NULL;
+ int rw;
+
+ rw = mode & (CRYPTO_READ|CRYPTO_WRITE);
+ if (!((rw == CRYPTO_READ) || (rw == CRYPTO_WRITE)))
+ {
+ errstr = "invalid mode";
+ goto err;
+ }
+
+ if (type < 0 || type > CRYPTO_NUM_LOCKS)
+ {
+ errstr = "type out of bounds";
+ goto err;
+ }
+
+ if (mode & CRYPTO_LOCK)
+ {
+ if (modes[type])
+ {
+ errstr = "already locked";
+ /* must not happen in a single-threaded program
+ * (would deadlock) */
+ goto err;
+ }
+
+ modes[type] = rw;
+ }
+ else if (mode & CRYPTO_UNLOCK)
+ {
+ if (!modes[type])
+ {
+ errstr = "not locked";
+ goto err;
+ }
+
+ if (modes[type] != rw)
+ {
+ errstr = (rw == CRYPTO_READ) ?
+ "CRYPTO_r_unlock on write lock" :
+ "CRYPTO_w_unlock on read lock";
+ }
+
+ modes[type] = 0;
+ }
+ else
+ {
+ errstr = "invalid mode";
+ goto err;
+ }
+
+ err:
+ if (errstr)
+ {
+ /* we cannot use bio_err here */
+ fprintf(stderr, "openssl (lock_dbg_cb): %s (mode=%d, type=%d) at %s:%d\n",
+ errstr, mode, type, file, line);
+ }
+ }
+
+
int main(int Argc, char *Argv[])
{
ARGS arg;
@@ -112,6 +231,13 @@
CRYPTO_malloc_debug_init();
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+#if 0
+ if (getenv("OPENSSL_DEBUG_LOCKING") != NULL)
+#endif
+ {
+ CRYPTO_set_locking_callback(lock_dbg_cb);
+ }
+
apps_startup();
if (bio_err == NULL)
diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c
index 9de60fd..8634c07 100644
--- a/crypto/cryptlib.c
+++ b/crypto/cryptlib.c
@@ -133,11 +133,11 @@
char *str;
int i;
+#if defined(WIN32) || defined(WIN16)
/* A hack to make Visual C++ 5.0 work correctly when linking as
* a DLL using /MT. Without this, the application cannot use
* and floating point printf's.
* It also seems to be needed for Visual C 1.5 (win16) */
-#if defined(WIN32) || defined(WIN16)
SSLeay_MSVC5_hack=(double)name[0]*(double)name[1];
#endif
diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 35ea2c2..3898c33 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -1,4 +1,17 @@
/* crypto/ex_data.c */
+
+/*
+ * This is not thread-safe, nor can it be changed to become thread-safe
+ * without changing various function prototypes and using a lot of locking.
+ * Luckily, it's not really used anywhere except in ssl_verify_cert_chain
+ * via SSL_get_ex_data_X509_STORE_CTX_idx (ssl/ssl_cert.c), where
+ * new_func, dup_func, and free_func all are 0.
+ *
+ * Any multi-threaded application crazy enough to use ex_data for its own
+ * purposes had better make sure that SSL_get_ex_data_X509_STORE_CTX_idx
+ * is called once before multiple threads are created.
+ */
+
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c
index 9ed8184..a6c70e4 100644
--- a/crypto/mem_dbg.c
+++ b/crypto/mem_dbg.c
@@ -678,7 +678,15 @@
* void_fn_to_char kludge in CRYPTO_mem_leaks_cb.
* Otherwise the code police will come and get us.)
*/
+ int old_mh_mode;
+
CRYPTO_w_lock(CRYPTO_LOCK_MALLOC);
+
+ /* avoid deadlock when lh_free() uses CRYPTO_dbg_free(),
+ * which uses CRYPTO_is_mem_check_on */
+ old_mh_mode = mh_mode;
+ mh_mode = CRYPTO_MEM_CHECK_OFF;
+
if (mh != NULL)
{
lh_free(mh);
@@ -692,6 +700,8 @@
amih = NULL;
}
}
+
+ mh_mode = old_mh_mode;
CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC);
}
MemCheck_on(); /* releases CRYPTO_LOCK_MALLOC2 */
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 0f4110c..32515cb 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -80,10 +80,7 @@
static STACK_OF(CRYPTO_EX_DATA_FUNCS) *x509_store_ctx_method=NULL;
static int x509_store_ctx_num=0;
-#if 0
-static int x509_store_num=1;
-static STACK *x509_store_method=NULL;
-#endif
+
static int null_callback(int ok, X509_STORE_CTX *e)
{
@@ -702,12 +699,17 @@
int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func)
- {
- x509_store_ctx_num++;
- return CRYPTO_get_ex_new_index(x509_store_ctx_num-1,
+ {
+ /* This function is (usually) called only once, by
+ * SSL_get_ex_data_X509_STORE_CTX_idx (ssl/ssl_cert.c).
+ * That function uses locking, so we don't (usually)
+ * have to worry about locking here. For the whole cruel
+ * truth, see crypto/ex_data.c */
+ x509_store_ctx_num++;
+ return CRYPTO_get_ex_new_index(x509_store_ctx_num-1,
&x509_store_ctx_method,
- argl,argp,new_func,dup_func,free_func);
- }
+ argl,argp,new_func,dup_func,free_func);
+ }
int X509_STORE_CTX_set_ex_data(X509_STORE_CTX *ctx, int idx, void *data)
{
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 130bb79..85d58c8 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -129,15 +129,23 @@
int SSL_get_ex_data_X509_STORE_CTX_idx(void)
{
- static int ssl_x509_store_ctx_idx= -1;
+ static volatile int ssl_x509_store_ctx_idx= -1;
- /* FIXME: should do locking */
if (ssl_x509_store_ctx_idx < 0)
{
- ssl_x509_store_ctx_idx=X509_STORE_CTX_get_ex_new_index(
- 0,"SSL for verify callback",NULL,NULL,NULL);
+ /* any write lock will do; usually this branch
+ * will only be taken once anyway */
+ CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX);
+
+ if (ssl_x509_store_ctx_idx < 0)
+ {
+ ssl_x509_store_ctx_idx=X509_STORE_CTX_get_ex_new_index(
+ 0,"SSL for verify callback",NULL,NULL,NULL);
+ }
+
+ CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX);
}
- return(ssl_x509_store_ctx_idx);
+ return ssl_x509_store_ctx_idx;
}
CERT *ssl_cert_new(void)
@@ -452,13 +460,15 @@
if (SSL_get_verify_depth(s) >= 0)
X509_STORE_CTX_set_depth(&ctx, SSL_get_verify_depth(s));
X509_STORE_CTX_set_ex_data(&ctx,SSL_get_ex_data_X509_STORE_CTX_idx(),s);
+
/* We need to set the verify purpose. The purpose can be determined by
* the context: if its a server it will verify SSL client certificates
* or vice versa.
- */
-
- if(s->server) i = X509_PURPOSE_SSL_CLIENT;
- else i = X509_PURPOSE_SSL_SERVER;
+ */
+ if (s->server)
+ i = X509_PURPOSE_SSL_CLIENT;
+ else
+ i = X509_PURPOSE_SSL_SERVER;
X509_STORE_CTX_purpose_inherit(&ctx, i, s->purpose, s->trust);
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index 77ac362..9d513ba 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -55,6 +55,59 @@
* copied and put under another distribution licence
* [including the GNU Public Licence.]
*/
+/* ====================================================================
+ * Copyright (c) 1998-2000 The OpenSSL Project. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ * software must display the following acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit. (http://www.openssl.org/)"
+ *
+ * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
+ * endorse or promote products derived from this software without
+ * prior written permission. For written permission, please contact
+ * openssl-core@openssl.org.
+ *
+ * 5. Products derived from this software may not be called "OpenSSL"
+ * nor may "OpenSSL" appear in their names without prior written
+ * permission of the OpenSSL Project.
+ *
+ * 6. Redistributions of any form whatsoever must retain the following
+ * acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit (http://www.openssl.org/)"
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This product includes cryptographic software written by Eric Young
+ * (eay@cryptsoft.com). This product includes software written by Tim
+ * Hudson (tjh@cryptsoft.com).
+ *
+ */
#include <assert.h>
#include <errno.h>
@@ -202,6 +255,69 @@
BIO_printf(bio_stdout,"\n");
}
+static void lock_dbg_cb(int mode, int type, const char *file, int line)
+ {
+ static int modes[CRYPTO_NUM_LOCKS]; /* = {0, 0, ... } */
+ const char *errstr = NULL;
+ int rw;
+
+ rw = mode & (CRYPTO_READ|CRYPTO_WRITE);
+ if (!((rw == CRYPTO_READ) || (rw == CRYPTO_WRITE)))
+ {
+ errstr = "invalid mode";
+ goto err;
+ }
+
+ if (type < 0 || type > CRYPTO_NUM_LOCKS)
+ {
+ errstr = "type out of bounds";
+ goto err;
+ }
+
+ if (mode & CRYPTO_LOCK)
+ {
+ if (modes[type])
+ {
+ errstr = "already locked";
+ /* must not happen in a single-threaded program
+ * (would deadlock) */
+ goto err;
+ }
+
+ modes[type] = rw;
+ }
+ else if (mode & CRYPTO_UNLOCK)
+ {
+ if (!modes[type])
+ {
+ errstr = "not locked";
+ goto err;
+ }
+
+ if (modes[type] != rw)
+ {
+ errstr = (rw == CRYPTO_READ) ?
+ "CRYPTO_r_unlock on write lock" :
+ "CRYPTO_w_unlock on read lock";
+ }
+
+ modes[type] = 0;
+ }
+ else
+ {
+ errstr = "invalid mode";
+ goto err;
+ }
+
+ err:
+ if (errstr)
+ {
+ /* we cannot use bio_err here */
+ fprintf(stderr, "openssl (lock_dbg_cb): %s (mode=%d, type=%d) at %s:%d\n",
+ errstr, mode, type, file, line);
+ }
+ }
+
int main(int argc, char *argv[])
{
char *CApath=NULL,*CAfile=NULL;
@@ -235,6 +351,8 @@
debug = 0;
cipher = 0;
+ CRYPTO_set_locking_callback(lock_dbg_cb);
+
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
RAND_seed(rnd_seed, sizeof rnd_seed);
