Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Wed, Sep 25, 2002 at 09:22:20PM +0200, Patrick McCormick via RT wrote: I was looking at some other code in the ssl directory, and the *_method functions in the *_meth.c files appear to use the same initialization idiom I believe they need to be thread-protected also. Fixed. -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
I wrote that the next snapshots should solve the problem. Functions
SSLv23_client_method(), SSLv23_server_method(),
SSLv2_client_method(),SSLv2_server_method(),
SSLv3_client_method(),SSLv3_server_method(),
TLSv1_client_method(),TLSv1_server_method()
now use a lock.
Bodo,
Many thanks for putting in a lock. However, the race condition has not been
eliminated.
The functions now have:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
memcpy((char *)SSLv3_client_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_client_data.ssl_connect=ssl3_connect;
SSLv3_client_data.get_ssl_method=ssl3_get_client_method;
init=0;
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
Two threads can enter init, and stop at the lock. One thread will get the
lock, set up client_data, exit the lock, and start using the struct. Then,
the other thread will get the lock, and mangle client_data while the first
thread is using it.
To prevent this, init must be checked after the lock is entered in order to
prevent the client_data setup from happening twice. So, add:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
if (init)
{
}
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
Yes, the if (test) lock() if (test) construct is very important (although I thought I saw a bug about JVM implementations getting this wrong). Anyone doing threads programming should read Andrew Birrell's 1989 tutorial. http://gatekeeper.dec.com/pub/DEC/SRC/research-reports/abstracts/src-rr-035.html FYI, Birrell invented RPC. /r$ __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Tue, Sep 24, 2002 at 03:47:14PM -0700, Patrick McCormick wrote:
Many thanks for putting in a lock. However, the race condition has not been
eliminated.
[...]init must be checked after the lock is entered in order to
prevent the client_data setup from happening twice. So, add:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
if (init)
{
}
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
You are absolutely right, of course. I got similar constructs right
in the past, but I guess this time I was too busy with various other
things to really think about what I was writing ...
The next snapshot should really fix the problem.
--
Bodo Möller [EMAIL PROTECTED]
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Tue, Sep 24, 2002 at 03:47:14PM -0700, Patrick McCormick wrote:
Many thanks for putting in a lock. However, the race condition has not been
eliminated.
[...]init must be checked after the lock is entered in order to
prevent the client_data setup from happening twice. So, add:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
if (init)
{
}
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
You are absolutely right, of course. I got similar constructs right
in the past, but I guess this time I was too busy with various other
things to really think about what I was writing ...
The next snapshot should really fix the problem.
--
Bodo Möller [EMAIL PROTECTED]
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
You are absolutely right, of course. I got similar constructs right in the past, but I guess this time I was too busy with various other things to really think about what I was writing ... The next snapshot should really fix the problem. I looked at the changes in CVS and everything seems to be correct now. Thanks for fixing this so quickly. I was looking at some other code in the ssl directory, and the *_method functions in the *_meth.c files appear to use the same initialization idiom. I believe they need to be thread-protected also. I grepped for if (init), and it now appears twice in all files except for *_meth.c and ssl_err.c. I don't believe ssl_err needs to be threadsafe since there's a thread safety page (somewhere, openssl.org is real slow right now) that explicitly says to run the SSL_load_error_strings() before using openssl on multiple threads. patrick __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Mon, Sep 23, 2002 at 06:43:21PM +0200, Bodo Moeller wrote: On Mon, Sep 23, 2002 at 04:26:00PM +0200, Bodo Moeller via RT wrote: Somehow the RT2 system doesn't like the comments I enter at the website when resolving a ticket ... I am not yet sure (would need to do a test), but it seems that RT2 eats the last line that is not terminated by a newline. I just ate my Lutz from my usual Best regards,\nLutz. Fortunately, it did not eat up more important parts of the text... Best regards, Lutz -- Lutz Jaenicke [EMAIL PROTECTED] http://www.aet.TU-Cottbus.DE/personen/jaenicke/ BTU Cottbus, Allgemeine Elektrotechnik Universitaetsplatz 3-4, D-03044 Cottbus __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
You are absolutely right, of course. I got similar constructs right in the past, but I guess this time I was too busy with various other things to really think about what I was writing ... The next snapshot should really fix the problem. I looked at the changes in CVS and everything seems to be correct now. Thanks for fixing this so quickly. I was looking at some other code in the ssl directory, and the *_method functions in the *_meth.c files appear to use the same initialization idiom. I believe they need to be thread-protected also. I grepped for if (init), and it now appears twice in all files except for *_meth.c and ssl_err.c. I don't believe ssl_err needs to be threadsafe since there's a thread safety page (somewhere, openssl.org is real slow right now) that explicitly says to run the SSL_load_error_strings() before using openssl on multiple threads. patrick __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
I wrote that the next snapshots should solve the problem. Functions
SSLv23_client_method(), SSLv23_server_method(),
SSLv2_client_method(),SSLv2_server_method(),
SSLv3_client_method(),SSLv3_server_method(),
TLSv1_client_method(),TLSv1_server_method()
now use a lock.
Bodo,
Many thanks for putting in a lock. However, the race condition has not been
eliminated.
The functions now have:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
memcpy((char *)SSLv3_client_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_client_data.ssl_connect=ssl3_connect;
SSLv3_client_data.get_ssl_method=ssl3_get_client_method;
init=0;
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
Two threads can enter init, and stop at the lock. One thread will get the
lock, set up client_data, exit the lock, and start using the struct. Then,
the other thread will get the lock, and mangle client_data while the first
thread is using it.
To prevent this, init must be checked after the lock is entered in order to
prevent the client_data setup from happening twice. So, add:
if (init)
{
CRYPTO_w_lock(CRYPTO_LOCK_SSL_METHOD);
if (init)
{
}
CRYPTO_w_unlock(CRYPTO_LOCK_SSL_METHOD);
}
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Fri, Sep 20, 2002 at 06:19:48PM -0700, Patrick McCormick wrote: Here's one step by step scenario. You are absolutely right about the bug. I somehow had not realized that the memcpy accesses the same struct as the following assignments. We need a lock to fix this. -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #262] bug: init race in SSLv3_client_method
__ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Mon, Sep 23, 2002 at 04:26:00PM +0200, Bodo Moeller via RT wrote: Somehow the RT2 system doesn't like the comments I enter at the website when resolving a ticket ... I wrote that the next snapshots should solve the problem. Functions SSLv23_client_method(), SSLv23_server_method(), SSLv2_client_method(),SSLv2_server_method(), SSLv3_client_method(),SSLv3_server_method(), TLSv1_client_method(),TLSv1_server_method() now use a lock. -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment.
Can you elaborate? In such cases the other thread should execute the
'if' body too. A potential problem is not about atomicity, but about
reordering of statements (if the assignment to 'init' happens before
on of the other assignements, we have a problem), so it might be
better to make those static variables 'volatile'.
Here's one step by step scenario.
Thread T1 enters the if block, does the memcpy, does the assignments, and
then a context switch occurs.
Thread T2 enters the if block (since init is still 1) and a context switch
occurs.
Thread T1 sets init=0, exits the function, and a context switch occurs.
Thread T2 performs the memcpy. Context switch.
Thread T1 does some stuff, and eventually calls SSL_set_session, which calls
get_ssl_method. Because get_ssl_method has been reinitialized to
ssl_undefined_function by the memcpy, this method errors out.
There are other scenarios to exploit this race condition. The bottom line
is that you must either use a single atomic function to alter static data,
or a lock around a group of nonatomic operations.
(I'm aware that the code still is bad in theory, but it should work
for all implementations. There's more stuff like that in OpenSSL, but
I can't rewrite all of it ...)
The thread-unsafe code in OpenSSL should be fixed, or you should just advise
users that while OpenSSL happens to work in most cases so far, the code is
not threadsafe.
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
All (most?) similar cases clear the 'init' flag *after* having set up
the data structures appropriately, e.g. see ssl/s3_meth.c.
Yes, SSLv3_client_method is the only one I saw which had init set in the
wrong place. I may have missed some.
No locking should be needed because the assignments are idempotent.
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment. In this case, ssl_undefined_function is
called and it errors out.
To make this code properly thread-safe, locks and atomic sets should be used
to protect any non-atomic functions working on shared data.
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Thu, Sep 19, 2002 at 06:28:16PM -0700, Patrick McCormick wrote:
No locking should be needed because the assignments are idempotent.
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment.
Can you elaborate? In such cases the other thread should execute the
'if' body too. A potential problem is not about atomicity, but about
reordering of statements (if the assignment to 'init' happens before
on of the other assignements, we have a problem), so it might be
better to make those static variables 'volatile'.
(I'm aware that the code still is bad in theory, but it should work
for all implementations. There's more stuff like that in OpenSSL, but
I can't rewrite all of it ...)
--
Bodo Möller [EMAIL PROTECTED]
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Thu, Sep 19, 2002 at 06:28:16PM -0700, Patrick McCormick wrote:
No locking should be needed because the assignments are idempotent.
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment.
Can you elaborate? In such cases the other thread should execute the
'if' body too. A potential problem is not about atomicity, but about
reordering of statements (if the assignment to 'init' happens before
on of the other assignements, we have a problem), so it might be
better to make those static variables 'volatile'.
(I'm aware that the code still is bad in theory, but it should work
for all implementations. There's more stuff like that in OpenSSL, but
I can't rewrite all of it ...)
--
Bodo Möller [EMAIL PROTECTED]
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Tue, Sep 03, 2002 at 05:29:41PM -0700, Patrick McCormick wrote:
I needed to add the following calls in my single-thread openssl setup code
to end several race conditions:
SSLv23_client_method();
SSLv2_client_method();
SSLv3_client_method();
TLSv1_client_method();
SSLv23_method();
SSLv2_method();
SSLv3_method();
TLSv1_method();
SSLv23_server_method();
SSLv2_server_method();
SSLv3_server_method();
TLSv1_server_method();
ssl2_get_cipher_by_char(XXX);
ssl3_get_cipher_by_char(XXX);
These functions appear to follow the same pattern (the
..._get_cipher_by_char functions actually use locks because they do
more than just simple assignments and copying).
I also see race conditions in crypto/rand/md_rand.c that I don't see a
workaround for. There's an init race in ssleay_rand_bytes. Multiple
threads can call in and end up in the initialization code if init==0. I'm
not sure why there is a lock around if (init), since this lock doesn't
prevent multiple initialization.
These threads then both call RAND_seed (ssleay_rand_seed, for me) at once.
ssleay_rand_seed modifies globals without any locking, so it doesn't appear
thread safe.
Er, what version of OpenSSL are you looking at? In 0.9.6g, we have
static int ssleay_rand_bytes(unsigned char *buf, int num)
{
[...]
CRYPTO_w_lock(CRYPTO_LOCK_RAND);
/* prevent ssleay_rand_bytes() from trying to obtain the lock again */
CRYPTO_w_lock(CRYPTO_LOCK_RAND2);
locking_thread = CRYPTO_thread_id();
CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
crypto_lock_rand = 1;
if (!initialized)
{
RAND_poll();
initialized = 1;
}
so the call to RAND_poll() (and eventually to ssleay_rand_see())
happens inside the write lock, meaing that only a single thread can do
this at a time.
ssleay_rand_bytes modifies globals (md, md_count, etc.) without locking, so
the random byte buffer can be filled from md while another thread is
rewriting the contents of md. Also, md_count[0]++ has undefined
results.
md_count[0] += 1 and access to md happens while the thread has the
CRYPTO_LOCK_RAND lock. Some accesses to the state array are not
protected by locking, however, because it does not really matter which
thread wins.
--
Bodo Möller [EMAIL PROTECTED]
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment.
Can you elaborate? In such cases the other thread should execute the
'if' body too. A potential problem is not about atomicity, but about
reordering of statements (if the assignment to 'init' happens before
on of the other assignements, we have a problem), so it might be
better to make those static variables 'volatile'.
Here's one step by step scenario.
Thread T1 enters the if block, does the memcpy, does the assignments, and
then a context switch occurs.
Thread T2 enters the if block (since init is still 1) and a context switch
occurs.
Thread T1 sets init=0, exits the function, and a context switch occurs.
Thread T2 performs the memcpy. Context switch.
Thread T1 does some stuff, and eventually calls SSL_set_session, which calls
get_ssl_method. Because get_ssl_method has been reinitialized to
ssl_undefined_function by the memcpy, this method errors out.
There are other scenarios to exploit this race condition. The bottom line
is that you must either use a single atomic function to alter static data,
or a lock around a group of nonatomic operations.
(I'm aware that the code still is bad in theory, but it should work
for all implementations. There's more stuff like that in OpenSSL, but
I can't rewrite all of it ...)
The thread-unsafe code in OpenSSL should be fixed, or you should just advise
users that while OpenSSL happens to work in most cases so far, the code is
not threadsafe.
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
[openssl.org #262] bug: init race in SSLv3_client_method
All (most?) similar cases clear the 'init' flag *after* having set up the data structures appropriately, e.g. see ssl/s3_meth.c. No locking should be needed because the assignments are idempotent. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
All (most?) similar cases clear the 'init' flag *after* having set up
the data structures appropriately, e.g. see ssl/s3_meth.c.
Yes, SSLv3_client_method is the only one I saw which had init set in the
wrong place. I may have missed some.
No locking should be needed because the assignments are idempotent.
However, the assignments are not atomic. The following unprotected
operation:
if (init)
{
memcpy((char *)SSLv3_server_data,(char *)sslv3_base_method(),
sizeof(SSL_METHOD));
SSLv3_server_data.ssl_accept=ssl3_accept;
SSLv3_server_data.get_ssl_method=ssl3_get_server_method;
init=0;
}
can result in a thread calling .ssl_accept or .get_ssl_method after the
memcpy but before the assignment. In this case, ssl_undefined_function is
called and it errors out.
To make this code properly thread-safe, locks and atomic sets should be used
to protect any non-atomic functions working on shared data.
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
Normally application create a single SSL_CTX structure before starting any threads and muliple SSL structure from that SSL_CTX so that problem doesn't arise. In my app, multiple threads call into the *_client_method structures because they don't know ahead of time which transport to use. Possibly someone could add a note to the threads manpage that advises apps to call all the methods below initially on a single thread. I suppose it should be locked though. A simple workaround is to make a dummy SSL_v3_client_method() call before starting any threads. This is the same workaround I eventually came up with. I needed to add the following calls in my single-thread openssl setup code to end several race conditions: SSLv23_client_method(); SSLv2_client_method(); SSLv3_client_method(); TLSv1_client_method(); SSLv23_method(); SSLv2_method(); SSLv3_method(); TLSv1_method(); SSLv23_server_method(); SSLv2_server_method(); SSLv3_server_method(); TLSv1_server_method(); ssl2_get_cipher_by_char(XXX); ssl3_get_cipher_by_char(XXX); I also see race conditions in crypto/rand/md_rand.c that I don't see a workaround for. There's an init race in ssleay_rand_bytes. Multiple threads can call in and end up in the initialization code if init==0. I'm not sure why there is a lock around if (init), since this lock doesn't prevent multiple initialization. These threads then both call RAND_seed (ssleay_rand_seed, for me) at once. ssleay_rand_seed modifies globals without any locking, so it doesn't appear thread safe. ssleay_rand_bytes modifies globals (md, md_count, etc.) without locking, so the random byte buffer can be filled from md while another thread is rewriting the contents of md. Also, md_count[0]++ has undefined results. I see that access to state_num and state_index is locked, so I'm confused why the other globals are modified outside of locks. Thanks for looking at this! Patrick __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #262] bug: init race in SSLv3_client_method
On Sun, Sep 01, 2002, Patrick McCormick via RT wrote:
In this method in ssl/s3_clnt.c, there's a race condition with the static
init variable that is causing a crash in my multithreaded program. init
gets set to 0 before the static structures have been set up. I believe a
lock is needed.
142 SSL_METHOD *SSLv3_client_method(void)
143 {
144 static int init=1;
145 static SSL_METHOD SSLv3_client_data;
146
147 if (init)
148 {
149 init=0;
150 memcpy((char *)SSLv3_client_data,(char
*)sslv3_base_method(),
151 sizeof(SSL_METHOD));
152 SSLv3_client_data.ssl_connect=ssl3_connect;
153 SSLv3_client_data.get_ssl_method=ssl3_get_client_method;
154 }
155 return(SSLv3_client_data);
156 }
Has anyone run into this before? What method does OpenSSL use to lock
access to data structures?
Normally application create a single SSL_CTX structure before starting
any threads and muliple SSL structure from that SSL_CTX so that problem
doesn't arise.
I suppose it should be locked though. A simple workaround is to make a
dummy SSL_v3_client_method() call before starting any threads.
Steve.
--
Dr. Stephen Henson [EMAIL PROTECTED]
OpenSSL Project http://www.openssl.org/~steve/
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
[openssl.org #262] bug: init race in SSLv3_client_method
In this method in ssl/s3_clnt.c, there's a race condition with the static
init variable that is causing a crash in my multithreaded program. init
gets set to 0 before the static structures have been set up. I believe a
lock is needed.
142 SSL_METHOD *SSLv3_client_method(void)
143 {
144 static int init=1;
145 static SSL_METHOD SSLv3_client_data;
146
147 if (init)
148 {
149 init=0;
150 memcpy((char *)SSLv3_client_data,(char
*)sslv3_base_method(),
151 sizeof(SSL_METHOD));
152 SSLv3_client_data.ssl_connect=ssl3_connect;
153 SSLv3_client_data.get_ssl_method=ssl3_get_client_method;
154 }
155 return(SSLv3_client_data);
156 }
Has anyone run into this before? What method does OpenSSL use to lock
access to data structures?
thanks,
patrick
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
