Full_Name: Doug Leavitt Version: 2.4.44 OS: Solaris URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (137.254.4.13) There is a race condition in ldap_int_utils_init that can be triggered when multiple threads enter ldap_int_utils_init from ldap_init_initialize about the same time. The done flag gets set immediately, before the various mutexes are initialized. If thread A sets done, and thread B tests for done==1 before thread A has completed the mutex inits, thread B can attempt to use an uninitialized mutex and fail/core dump etc. Additionally if judt the done=1 is moved to the bottom of the function thwo threads can both be initializing the same mutexes multiple times causes other mayhem. The short term workaround for Solaris (THR APIs) is to move setting of done=1 to after the mutex inits, and to protect the mutex inits using another statically initialized mutex within ldap_int_utils_init. Example patch: -- util-int.c.~1~ 2016-02-05 17:57:45.000000000 -0600 +++ util-int.c 2016-06-24 13:20:09.793308570 -0500 @@ -89,12 +89,16 @@ # endif # if defined(HAVE_GETHOSTBYADDR_R) && \ (GETHOSTBDDDDR_R_NARGS < 7) || (8 < GETHOSTBYADDR_R_NARGS) /* Don't know how to handle this version, pretend it's not there */ # undef HAVE_GETHOSTBYADDR_R # endif +#if defined( HAVE_THR ) + /* use static mutex to protect mutex initialization */ +static mutex_t ldap_int_utils_init_mutex = DEFAULTMUTEX; +#endif /* HAVE_THR */ #endif /* LDAP_R_COMPILE */ char *ldap_pvt_ctime( const time_t *tp, char *buf ) { #ifdef USE_CTIME_R # if (CTIME_R_NARGS > 3) || (CTIME_R_NARGS < 2) @@69697,15 +701,22 @@ void ldap_int_utils_init( void ) { static int done=0; if (done) return; - done=1; #ifdef LDAP_R_COMPILE +#if defined( HAVE_THR ) + /* use static mutex to protect mutex initialization */ + (void) mutex_lock(&ldap_int_utils_init_mutex); + if (done) { + (void) mutex_unlock(&ldap_int_utils_init_mutex); + return; + } +#endif /* HAVE_THR */ #if !defined( USE_CTIME_R ) && !defined( HAVE_REENTRANT_FUNCTIONS ) ldap_pvt_thread_mutex_init( &ldap_int_ctime_mutex ); #endif #if !defined( USE_GMTIME_R ) && !defined( USE_LOCALTIME_R ) ldap_pvt_thread_mutex_init( &ldap_int_gmtime_mutex ); #endif @@ -715,15 +726,20 @@ ldap_pvt_thread_mutex_init( &ldap_int_gettime_mutex ); #ifdef HAVE_GSSAPI ldap_pvt_thread_mutex_init( &ldap_int_gssapi_mutex ); #endif -#endif /* call other module init functions here... */ + done=1; +#if defined( HAVE_THR ) + /* use static mutex to protect mutex initialization */ + (void) mutex_unlock(&ldap_int_utils_init_mutex); +#endif /* HAVE_THR */ +#endif } #if defined( NEED_COPY_HOSTENT ) # undef NEED_SAFE_REALLOC #define NEED_SAFE_REALLOC I know a similar workaround could be made for POSIX threads and possibly some of the other supported thread models, but this approach seems kludgely. I suspect the correct solution may be to somehow refactor ldap_int_utils_init out of libldap and into libldap_r in a cleaner multi-platform manner than the example above.
doug.leavitt@oracle.com wrote: > Full_Name: Doug Leavitt > Version: 2.4.44 > OS: Solaris > URL: ftp://ftp.openldap.org/incoming/ > Submission from: (NULL) (137.254.4.13) > > > There is a race condition in ldap_int_utils_init that can be triggered when > multiple threads enter ldap_int_utils_init from ldap_init_initialize about the > same time. The done flag gets set immediately, before the various mutexes are > initialized. If thread A sets done, and thread B tests for done==1 before thread > A has completed the mutex inits, thread B can attempt to use an uninitialized > mutex and fail/core dump etc. > > Additionally if judt the done=1 is moved to the bottom of the function thwo > threads can both be initializing the same mutexes multiple times causes other > mayhem. > > The short term workaround for Solaris (THR APIs) is to move setting of done=1 to > after the mutex inits, and to protect the mutex inits using another statically > initialized mutex within ldap_int_utils_init. > I know a similar workaround could be made for POSIX threads and possibly some of > the other supported thread models, but this approach seems kludgely. Static initializers would be the simplest fix, certainly. On Windows we'd have to use something similar to pthread_once() for initialization, instead. > I suspect the correct solution may be to somehow refactor ldap_int_utils_init > out of libldap and into libldap_r in a cleaner multi-platform manner than the > example above. > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
moved from Incoming to Software Bugs
changed notes
On Fri, Jun 24, 2016 at 08:04:27PM +0000, doug.leavitt@oracle.com wrote: > There is a race condition in ldap_int_utils_init that can be triggered when > multiple threads enter ldap_int_utils_init from ldap_init_initialize about the > same time. The done flag gets set immediately, before the various mutexes are > initialized. If thread A sets done, and thread B tests for done==1 before thread > A has completed the mutex inits, thread B can attempt to use an uninitialized > mutex and fail/core dump etc. > > Additionally if judt the done=1 is moved to the bottom of the function thwo > threads can both be initializing the same mutexes multiple times causes other > mayhem. > > The short term workaround for Solaris (THR APIs) is to move setting of done=1 to > after the mutex inits, and to protect the mutex inits using another statically > initialized mutex within ldap_int_utils_init. Hi Doug, a patch addressing this and ITS#7996 has been pushed to master (db40120a276c3b7968552e253aea24860fad5f60) and will also be part (cde56fad154fcd25e351c3cd84d8173d263b0a01) of the upcoming 2.4.48 release. Thanks, -- Ondřej Kuzník Senior Software Engineer Symas Corporation http://www.symas.com Packaged, certified, and supported LDAP solutions powered by OpenLDAP
changed notes changed state Open to Release
Fixed in master Fixed in RE24 (2.4.48) via ITS#7996
changed notes changed state Release to Closed