httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geoff Thorpe <ge...@geoffthorpe.net>
Subject Re: OpenSSL locking was Re: mod_ssl broken as DSO in HEAD
Date Wed, 18 Jun 2003 15:32:57 GMT
Hi,

On June 18, 2003 02:28 am, Justin Erenkrantz wrote:
> Finally got a chance to dig some more and found that if I add:
>
> CRYPTO_set_id_callback(openssl_id);
>
> to ssl_hook_pre_config before the ENGINE_load_builtin_engines and where
> openssl_id is defined as:
>
> static unsigned long openssl_id(void)
> {
>     /* FIXME: This is lame and not portable. -aaron */
>     return (unsigned long) apr_os_thread_current();
> }
>
> (The above is used in flood - hence Aaron's comment.)
>
> The SEGVs go away if we do this.  Does this make sense given the
> internals of OpenSSL?

I'm not the best person to talk about with respect to locking in openssl 
(I more or less detest threads, an Alan Cox quote about state-machines 
springs to mind...). That said, :-), I took another look at the backtrace 
you posted and it certainly seems odd. Digging ... the implementation of 
CRYPTO_thread_id() is;

unsigned long CRYPTO_thread_id(void)
        {
        unsigned long ret=0;

        if (id_callback == NULL)
                {
#ifdef OPENSSL_SYS_WIN16
                ret=(unsigned long)GetCurrentTask();
#elif defined(OPENSSL_SYS_WIN32)
                ret=(unsigned long)GetCurrentThreadId();
#elif defined(GETPID_IS_MEANINGLESS)
                ret=1L;
#else
                ret=(unsigned long)getpid();
#endif
                }
        else
                ret=id_callback();
        return(ret);
        }

The segfault is (or appears to be, stack smashing possibilities aside) 
occuring in a function called from this, and the address is less than the 
other openssl library functions in the trace, but not by much. Which begs 
the question - is id_callback set to something (and if so, what?) or does 
the address correspond to getpid()? Unfortunately, I think the easiest 
way to see this would be to hack the above function (and perhaps others 
in crypto/cryptlib.c that deal with id_callback) to watch what is going 
on.

> On this same tangent, do we need to be doing all of the CRYPTO_<lock>
> stuff? I don't believe we are doing that.  And, I know in flood, we had
> lots of problems until we called them.  So, I think mod_ssl should be
> passing the lock structures - especially for worker MPM builds...

Whenever I build Apache, I typically build a version of openssl configured 
with "no-threads" so this sort of issue is implicitly ruled out. It also 
removes a few meaningless function calls given that (multi-forked, 
traditional) Apache doesn't need any thread-safe locking. However I know 
that isn't the answer to your question in general, as many people will be 
using the openssl libs bundled with their system and/or trying to use 
Apache2 in a threaded setup.

> While I've got someone's attention, I have some real issues in that
> both of the OpenSSL lock implementations (CRYPTO_set_locking_callback
> and CRYPTO_set_dynlock_*) require global variables to implement.  I
> don't think I can say 'ick' loud enough.
>
> What would the possibility/feasibility of passing application-specific
> opaque values through the CRYPTO_lock callbacks?  Say
>
> static void openssl_lock(int mode, int n, void *app, const char *file,
> int line)
>
> static CRYPTO_dynlock_value *ssl_dyn_create(void *app, const char*
> file, int line)
>
> We can have app passed into the CRYPTO_set functions - in the case of
> httpd, it would be the pools for ssl_dyn_create *or* the global array
> of locks that openssl_lock would require.  You don't know how much
> happier that would make me if we could do that.  globals are just so
> icky.  -- justin

For this and the previous question, the short answer is "maybe, but it 
doesn't solve the problem of working with existing versions".

In both cases, ie. the segfault you're seeing *and* the issue about 
attaching an opaque pointer to caller-provide locking callbacks, it would 
make sense to take that conversation over to openssl-dev. In particular, 
we should get any appropriate details into the request-tracking system. 
BTW: Once we're over there, Richard Levitte may be able to provide better 
comments than I on this locking stuff, particularly as the dynlock stuff 
was (IIRC) his creation.

Cheers,
Geoff

-- 
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/

Mime
View raw message