From 152d306ad87dfd6fc8992330d6f29a86fb81533b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 11 Jun 2024 10:38:18 -0700 Subject: [PATCH] avoid UB with self pthread_join() --- modules/libcom/src/osi/os/posix/osdThread.c | 46 +++++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/modules/libcom/src/osi/os/posix/osdThread.c b/modules/libcom/src/osi/os/posix/osdThread.c index 17dfacbc8..e9ea2dfbc 100644 --- a/modules/libcom/src/osi/os/posix/osdThread.c +++ b/modules/libcom/src/osi/os/posix/osdThread.c @@ -687,34 +687,54 @@ static epicsThreadOSD *createImplicit(void) void epicsThreadMustJoin(epicsThreadId id) { - void *ret = NULL; int status; + int prev; + epicsThreadId self; - if(!id) { + if(!id) return; - } else if(epicsAtomicCmpAndSwapIntT(&id->joinable, 1, 0)!=1) { - if(epicsThreadGetIdSelf()==id) { - errlogPrintf("Warning: %s thread self-join of unjoinable\n", id->name); + + prev = epicsAtomicCmpAndSwapIntT(&id->joinable, 1, 0); + self = epicsThreadGetIdSelf(); + + if(prev==0) { + /* join of unjoinable */ + if(self==id) { + errlogPrintf(ERL_WARNING ": %s thread self-join of unjoinable\n", id->name); + return; } else { /* try to error nicely, however in all likelihood de-ref of * 'id' has already caused SIGSEGV as we are racing thread exit, * which free's 'id'. */ - cantProceed("Error: %s thread not joinable.\n", id->name); + cantProceed(ERL_ERROR ": %s can't join unjoinable %s\n", self->name, id->name); } - return; + + } else if(prev!=1) { /* also not 0, the only other allowed value. */ + cantProceed(ERL_ERROR ": %s joins corrupt thread handle\n", self->name); } - status = pthread_join(id->tid, &ret); - if(status == EDEADLK) { - /* Thread can't join itself (directly or indirectly) - * so we detach instead. + /* from this point, we are responsible to either detach or join (or leak memory) */ + + if(self!=id) { + status = pthread_join(id->tid, NULL); + checkStatusOnce(status, "pthread_join"); + /* on error, continue and attempt to detach */ + + } else { + /* Thread self pthread_join() isn't portable. + * We choose to allow, and treat as detach. */ + status = EDEADLK; + } + + if(status) { status = pthread_detach(id->tid); checkStatusOnce(status, "pthread_detach"); - } else checkStatusOnce(status, "pthread_join"); - free_threadInfo(id); + } + + free_threadInfo(id); /* release joinable reference */ } LIBCOM_API void epicsStdCall epicsThreadSuspendSelf(void)