From 113076a009309362a59d0336a76e5b17a03fde8f Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 3 Mar 2016 11:19:22 -0600 Subject: [PATCH 1/7] Allow quotes in DBD argument strings --- src/registry/registerRecordDeviceDriver.pl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registry/registerRecordDeviceDriver.pl b/src/registry/registerRecordDeviceDriver.pl index 06698c867..dbb17fd09 100644 --- a/src/registry/registerRecordDeviceDriver.pl +++ b/src/registry/registerRecordDeviceDriver.pl @@ -28,24 +28,24 @@ $bldTop =~ s/([\\"])/\\\1/g; # escape back-slashes and double-quotes open(INP,"$file") or die "$! opening file"; while() { next if m/ ^ \s* \# /x; - if (m/ \b recordtype \s* \( \s* (\w+) \s* \) /x) { + if (m/ \b recordtype \s* \( \s* "?(\w+)"? \s* \) /x) { $recordType[$numberRecordType++] = $1; } - elsif (m/ \b device \s* \( \s* (\w+) \W+ \w+ \W+ (\w+) /x) { + elsif (m/ \b device \s* \( \s* "?(\w+)"? \W+ \w+ \W+ (\w+) /x) { $deviceRecordType[$numberDeviceSupport] = $1; $deviceSupport[$numberDeviceSupport] = $2; $numberDeviceSupport++; } - elsif (m/ \b driver \s* \( \s* (\w+) \s* \) /x) { + elsif (m/ \b driver \s* \( \s* "?(\w+)"? \s* \) /x) { $driverSupport[$numberDriverSupport++] = $1; } - elsif (m/ \b registrar \s* \( \s* (\w+) \s* \) /x) { + elsif (m/ \b registrar \s* \( \s* "?(\w+)"? \s* \) /x) { push @registrars, $1; } - elsif (m/ \b function \s* \( \s* (\w+) \s* \) /x) { + elsif (m/ \b function \s* \( \s* "?(\w+)"? \s* \) /x) { push @registrars, "register_func_$1"; } - elsif (m/ \b variable \s* \( \s* (\w+) \s* , \s* (\w+) \s* \) /x) { + elsif (m/ \b variable \s* \( \s* "?(\w+)"? \s* , \s* "?(\w+)"? \s* \) /x) { $varType{$1} = $2; push @variables, $1; } From 4e312b9f645410cd5d91a17a5524655bfa248c0b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 16 Mar 2016 14:40:37 -0400 Subject: [PATCH 2/7] libCom: exitWait() from thread exit handler corrupts stack The epicsThreadCallEntryPoint() function stores a pointer to a local variable in epicsThread::pWaitReleaseFlag. Calling epicsAtThreadExit::exitWait() from that thread's epicsAtThreadExit() handler writes to this pointer after epicsThreadCallEntryPoint() has returned. Thus corrupting the stack. Set pWaitReleaseFlag=NULL before return to prevent this. fixes lp:1558206 --- src/libCom/osi/epicsThread.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libCom/osi/epicsThread.cpp b/src/libCom/osi/epicsThread.cpp index ed016f47e..a4124c216 100644 --- a/src/libCom/osi/epicsThread.cpp +++ b/src/libCom/osi/epicsThread.cpp @@ -109,6 +109,7 @@ extern "C" void epicsThreadCallEntryPoint ( void * pPvt ) // once the terminated flag is set and we release the lock // then the "this" pointer must not be touched again } + pThread->pWaitReleaseFlag = NULL; } bool epicsThread::beginWait () throw () From 6b9bfb09a5c675c154131f90f8f9b5f4c299b707 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 16 Mar 2016 17:43:52 -0400 Subject: [PATCH 3/7] pWaitRelease in wrong place waitRelease==false indicates that pThread has not be delete'd --- src/libCom/osi/epicsThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libCom/osi/epicsThread.cpp b/src/libCom/osi/epicsThread.cpp index a4124c216..4053e55ce 100644 --- a/src/libCom/osi/epicsThread.cpp +++ b/src/libCom/osi/epicsThread.cpp @@ -104,12 +104,12 @@ extern "C" void epicsThreadCallEntryPoint ( void * pPvt ) } if ( ! waitRelease ) { epicsGuard < epicsMutex > guard ( pThread->mutex ); + pThread->pWaitReleaseFlag = NULL; pThread->terminated = true; pThread->exitEvent.signal (); // once the terminated flag is set and we release the lock // then the "this" pointer must not be touched again } - pThread->pWaitReleaseFlag = NULL; } bool epicsThread::beginWait () throw () From 3179e6579133765531d923edc721b246b3f0eacb Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 16 Mar 2016 18:15:14 -0500 Subject: [PATCH 4/7] Clean-up after lp:1558206 fix --- documentation/RELEASE_NOTES.html | 7 +++ src/libCom/osi/epicsThread.cpp | 95 +++++++++++++++----------------- src/libCom/osi/epicsThread.h | 4 +- 3 files changed, 54 insertions(+), 52 deletions(-) diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html index 33406f2c9..35f991391 100644 --- a/documentation/RELEASE_NOTES.html +++ b/documentation/RELEASE_NOTES.html @@ -13,6 +13,13 @@ +

Fixed stack corruption bug in epicsThread C++ API

+ +

The C++ interface to the epicsThread API could corrupt the stack on thread +exit in some rare circumstances, usually at program exit. This bug has been +fixed (Launchpad bug +#1558206).

+

RTEMS NTP Support Issue

On RTEMS the NTP Time Provider could in some circumstances get out of sync diff --git a/src/libCom/osi/epicsThread.cpp b/src/libCom/osi/epicsThread.cpp index 4053e55ce..dcd210d04 100644 --- a/src/libCom/osi/epicsThread.cpp +++ b/src/libCom/osi/epicsThread.cpp @@ -4,7 +4,7 @@ * Copyright (c) 2002 The Regents of the University of California, as * Operator of Los Alamos National Laboratory. * EPICS BASE is distributed subject to a Software License Agreement found -* in file LICENSE that is included with this distribution. +* in file LICENSE that is included with this distribution. \*************************************************************************/ // // $Revision-Id$ @@ -37,22 +37,22 @@ epicsThreadRunable::~epicsThreadRunable () {} void epicsThreadRunable::run () {} void epicsThreadRunable::show ( unsigned int ) const {} -class epicsThread :: unableToCreateThread : +class epicsThread :: unableToCreateThread : public exception { public: const char * what () const throw (); }; -const char * epicsThread :: +const char * epicsThread :: unableToCreateThread :: what () const throw () { return "unable to create thread"; } -void epicsThread :: printLastChanceExceptionMessage ( +void epicsThread :: printLastChanceExceptionMessage ( const char * pExceptionTypeName, const char * pExceptionContext ) -{ +{ char date[64]; try { epicsTime cur = epicsTime :: getCurrent (); @@ -63,52 +63,52 @@ void epicsThread :: printLastChanceExceptionMessage ( } char name [128]; epicsThreadGetName ( this->id, name, sizeof ( name ) ); - errlogPrintf ( + errlogPrintf ( "epicsThread: Unexpected C++ exception \"%s\" " "with type \"%s\" in thread \"%s\" at %s\n", pExceptionContext, pExceptionTypeName, name, date ); errlogFlush (); - // this should behave as the C++ implementation intends when an - // exception isnt handled. If users dont like this behavior, they - // can install an application specific unexpected handler. + // This behavior matches the C++ implementation when an exception + // isn't handled by the thread code. Users can install their own + // application-specific unexpected handler if preferred. std::unexpected (); } extern "C" void epicsThreadCallEntryPoint ( void * pPvt ) { - epicsThread * pThread = + epicsThread * pThread = static_cast ( pPvt ); - bool waitRelease = false; + bool threadDestroyed = false; try { - pThread->pWaitReleaseFlag = & waitRelease; + pThread->pThreadDestroyed = & threadDestroyed; if ( pThread->beginWait () ) { pThread->runable.run (); - // current thread may have run the destructor - // so must not touch the this pointer from - // here on down if waitRelease is true + // The run() routine may have destroyed the epicsThread + // object by now; pThread can only be used below here + // when the threadDestroyed flag is false. } } catch ( const epicsThread::exitException & ) { } catch ( std::exception & except ) { - if ( ! waitRelease ) { - pThread->printLastChanceExceptionMessage ( + if ( ! threadDestroyed ) { + pThread->printLastChanceExceptionMessage ( typeid ( except ).name (), except.what () ); } } catch ( ... ) { - if ( ! waitRelease ) { - pThread->printLastChanceExceptionMessage ( + if ( ! threadDestroyed ) { + pThread->printLastChanceExceptionMessage ( "catch ( ... )", "Non-standard C++ exception" ); } } - if ( ! waitRelease ) { + if ( ! threadDestroyed ) { epicsGuard < epicsMutex > guard ( pThread->mutex ); - pThread->pWaitReleaseFlag = NULL; + pThread->pThreadDestroyed = NULL; pThread->terminated = true; pThread->exitEvent.signal (); - // once the terminated flag is set and we release the lock - // then the "this" pointer must not be touched again + // After the terminated flag is set and guard's destructor + // releases the lock, pThread must never be used again. } } @@ -136,12 +136,12 @@ void epicsThread::exitWait () throw () bool epicsThread::exitWait ( const double delay ) throw () { try { - // if destructor is running in managed thread then of - // course we will not wait for the managed thread to - // exit + // When called (usually by a destructor) in the context of + // the managed thread we can't wait for the thread to exit. + // Set the threadDestroyed flag and return success. if ( this->isCurrentThread() ) { - if ( this->pWaitReleaseFlag ) { - *this->pWaitReleaseFlag = true; + if ( this->pThreadDestroyed ) { + *this->pThreadDestroyed = true; } return true; } @@ -158,14 +158,14 @@ bool epicsThread::exitWait ( const double delay ) throw () } } catch ( std :: exception & except ) { - errlogPrintf ( + errlogPrintf ( "epicsThread::exitWait(): Unexpected exception " - " \"%s\"\n", + " \"%s\"\n", except.what () ); epicsThreadSleep ( epicsMin ( delay, 5.0 ) ); } catch ( ... ) { - errlogPrintf ( + errlogPrintf ( "Non-standard unexpected exception in " "epicsThread::exitWait()\n" ); epicsThreadSleep ( epicsMin ( delay, 5.0 ) ); @@ -175,14 +175,14 @@ bool epicsThread::exitWait ( const double delay ) throw () return this->terminated; } -epicsThread::epicsThread ( +epicsThread::epicsThread ( epicsThreadRunable & runableIn, const char * pName, unsigned stackSize, unsigned priority ) : - runable ( runableIn ), id ( 0 ), pWaitReleaseFlag ( 0 ), + runable ( runableIn ), id ( 0 ), pThreadDestroyed ( 0 ), begin ( false ), cancel ( false ), terminated ( false ) { - this->id = epicsThreadCreate ( - pName, priority, stackSize, epicsThreadCallEntryPoint, + this->id = epicsThreadCreate ( + pName, priority, stackSize, epicsThreadCallEntryPoint, static_cast < void * > ( this ) ); if ( ! this->id ) { throw unableToCreateThread (); @@ -194,11 +194,11 @@ epicsThread::~epicsThread () throw () while ( ! this->exitWait ( 10.0 ) ) { char nameBuf [256]; this->getName ( nameBuf, sizeof ( nameBuf ) ); - fprintf ( stderr, + fprintf ( stderr, "epicsThread::~epicsThread(): " - "blocking for thread \"%s\" to exit\n", + "blocking for thread \"%s\" to exit\n", nameBuf ); - fprintf ( stderr, + fprintf ( stderr, "was epicsThread object destroyed before thread exit ?\n"); } } @@ -273,11 +273,6 @@ void epicsThread::sleep (double seconds) throw () epicsThreadSleep (seconds); } -//epicsThread & epicsThread::getSelf () -//{ -// return * static_cast ( epicsThreadGetIdSelf () ); -//} - const char *epicsThread::getNameSelf () throw () { return epicsThreadGetNameSelf (); @@ -304,7 +299,7 @@ void epicsThread :: show ( unsigned level ) const throw () if ( level > 0u ) { epicsThreadShow ( this->id, level - 1 ); if ( level > 1u ) { - ::printf ( "pWaitReleaseFlag = %p\n", this->pWaitReleaseFlag ); + ::printf ( "pThreadDestroyed = %p\n", this->pThreadDestroyed ); ::printf ( "begin = %c, cancel = %c, terminated = %c\n", this->begin ? 'T' : 'F', this->cancel ? 'T' : 'F', @@ -324,12 +319,12 @@ extern "C" { epicsThreadPrivateId okToBlockPrivate; static const int okToBlockNo = 0; static const int okToBlockYes = 1; - + static void epicsThreadOnceIdInit(void *) { okToBlockPrivate = epicsThreadPrivateCreate(); } - + int epicsShareAPI epicsThreadIsOkToBlock(void) { const int *pokToBlock; @@ -337,7 +332,7 @@ extern "C" { pokToBlock = (int *) epicsThreadPrivateGet(okToBlockPrivate); return (pokToBlock ? *pokToBlock : 0); } - + void epicsShareAPI epicsThreadSetOkToBlock(int isOkToBlock) { const int *pokToBlock; @@ -345,12 +340,12 @@ extern "C" { pokToBlock = (isOkToBlock) ? &okToBlockYes : &okToBlockNo; epicsThreadPrivateSet(okToBlockPrivate, (void *)pokToBlock); } - + epicsThreadId epicsShareAPI epicsThreadMustCreate ( const char *name, unsigned int priority, unsigned int stackSize, - EPICSTHREADFUNC funptr,void *parm) + EPICSTHREADFUNC funptr,void *parm) { - epicsThreadId id = epicsThreadCreate ( + epicsThreadId id = epicsThreadCreate ( name, priority, stackSize, funptr, parm ); assert ( id ); return id; diff --git a/src/libCom/osi/epicsThread.h b/src/libCom/osi/epicsThread.h index fa0a6bd70..2fe97b379 100644 --- a/src/libCom/osi/epicsThread.h +++ b/src/libCom/osi/epicsThread.h @@ -144,10 +144,10 @@ public: bool isCurrentThread () const throw (); bool operator == ( const epicsThread & ) const throw (); void show ( unsigned level ) const throw (); + /* these operate on the current thread */ static void suspendSelf () throw (); static void sleep (double seconds) throw (); - /* static epicsThread & getSelf (); */ static const char * getNameSelf () throw (); static bool isOkToBlock () throw (); static void setOkToBlock ( bool isOkToBlock ) throw (); @@ -160,7 +160,7 @@ private: epicsMutex mutex; epicsEvent event; epicsEvent exitEvent; - bool * pWaitReleaseFlag; + bool * pThreadDestroyed; bool begin; bool cancel; bool terminated; From 106fae3b26b85526cc82d067423b6ac6849f70df Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 29 Mar 2016 17:36:39 +0900 Subject: [PATCH 5/7] dbStatic: prevent overflow in dbPutString() The bounds check should be before the string copy. Also zero the last element out of paranoia (should already be zero). Fix lp:1563191 --- src/dbStatic/dbStaticLib.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/dbStatic/dbStaticLib.c b/src/dbStatic/dbStaticLib.c index ab87911f3..655a08536 100644 --- a/src/dbStatic/dbStaticLib.c +++ b/src/dbStatic/dbStaticLib.c @@ -2198,7 +2198,10 @@ long epicsShareAPI dbPutString(DBENTRY *pdbentry,const char *pstring) switch (pflddes->field_type) { case DBF_STRING: if(!pfield) return(S_dbLib_fieldNotFound); - strncpy((char *)pfield, pstring,pflddes->size); + if(strlen(pstring) >= (size_t)pflddes->size) return S_dbLib_strLen; + strncpy((char *)pfield, pstring, pflddes->size-1); + ((char *)pfield)[pflddes->size-1] = 0; + if((pflddes->special == SPC_CALC) && !stringHasMacro) { char rpcl[RPCL_LEN]; short err; @@ -2209,7 +2212,6 @@ long epicsShareAPI dbPutString(DBENTRY *pdbentry,const char *pstring) calcErrorStr(err), pstring); } } - if((short)strlen(pstring) >= pflddes->size) status = S_dbLib_strLen; break; case DBF_CHAR: From d2d637d0c2fe3758bc3f92dad23a7a824b63b77d Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Tue, 29 Mar 2016 12:04:08 -0500 Subject: [PATCH 6/7] Prevent string overflow in recGblInitConstantLink() --- src/db/recGbl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/db/recGbl.c b/src/db/recGbl.c index 1337811a7..c0035f067 100644 --- a/src/db/recGbl.c +++ b/src/db/recGbl.c @@ -167,7 +167,8 @@ int epicsShareAPI recGblInitConstantLink( if(!plink->value.constantStr) return(FALSE); switch(dbftype) { case DBF_STRING: - strcpy((char *)pdest,plink->value.constantStr); + strncpy((char *)pdest, plink->value.constantStr, MAX_STRING_SIZE - 1); + ((char *)pdest)[MAX_STRING_SIZE - 1] = 0; break; case DBF_CHAR : { epicsInt16 value; From 430da57a35de7421f6c4f6f71a89a145eb6833d0 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Wed, 30 Mar 2016 11:00:48 -0500 Subject: [PATCH 7/7] Release notes for lp:1563191 --- documentation/RELEASE_NOTES.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html index 35f991391..68463ab7e 100644 --- a/documentation/RELEASE_NOTES.html +++ b/documentation/RELEASE_NOTES.html @@ -13,6 +13,13 @@ +

String field buffer overflows

+ +

Two buffer overflow bugs that can crash the IOC have been fixed, caused by +initializing a string field with a value larger than the field size +(Launchpad bug +#1563191).

+

Fixed stack corruption bug in epicsThread C++ API

The C++ interface to the epicsThread API could corrupt the stack on thread