> > 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]
