From 0bb14db63828a98ff077f8fbd237422d2f77c70a Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Mon, 3 Aug 2020 16:29:39 -0500 Subject: [PATCH 1/2] uspace_rtapi_app: Ensure string is NUL-terminated --- src/rtapi/uspace_rtapi_app.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rtapi/uspace_rtapi_app.cc b/src/rtapi/uspace_rtapi_app.cc index 69a71e69b67..9103b9facb2 100644 --- a/src/rtapi/uspace_rtapi_app.cc +++ b/src/rtapi/uspace_rtapi_app.cc @@ -493,7 +493,7 @@ static int get_fifo_path(char *buf, size_t bufsize) { const char *s = get_fifo_path(); if(!s) return -1; - strncpy(buf, s, bufsize); + snprintf(buf, bufsize, "%s", s); return 0; } From 2886e77a82695e54a7a825ebbfc5718835d4f4eb Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Mon, 3 Aug 2020 16:30:57 -0500 Subject: [PATCH 2/2] uspace_rtapi_app: error-check use of pthread mutexes It has been suggested that the logic in Posix::wait could lead to an unlock of a not-locked mutex. If this happens at runtime, rtapi_app will now abort. Note that as this lock is only used for sim / non-rt situations, this cannot affect people running on hardware. By setting RTAPI_LOCK_FAIL in the environment, you can check that the checking works, by causing a "lock while already locked" type error. --- src/rtapi/uspace_rtapi_app.cc | 42 ++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/rtapi/uspace_rtapi_app.cc b/src/rtapi/uspace_rtapi_app.cc index 9103b9facb2..ab12750824a 100644 --- a/src/rtapi/uspace_rtapi_app.cc +++ b/src/rtapi/uspace_rtapi_app.cc @@ -600,8 +600,12 @@ struct Posix : RtapiApp { Posix(int policy = SCHED_FIFO) : RtapiApp(policy), do_thread_lock(policy != SCHED_FIFO) { pthread_once(&key_once, init_key); - if(do_thread_lock) - pthread_mutex_init(&thread_lock, 0); + if(do_thread_lock) { + pthread_mutexattr_t attr; + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); + pthread_mutex_init(&thread_lock, &attr); + } } int task_delete(int id); int task_start(int task_id, unsigned long period_nsec); @@ -1042,9 +1046,21 @@ void *Posix::wrapper(void *arg) pthread_setspecific(key, arg); Posix &papp = reinterpret_cast(App()); - if(papp.do_thread_lock) - pthread_mutex_lock(&papp.thread_lock); + if(papp.do_thread_lock) { + int r = pthread_mutex_lock(&papp.thread_lock); + if(r != 0) { + printf("pthread_mutex_lock failed with status %d\n", r); + abort(); + } + } + if(papp.do_thread_lock && getenv("RTAPI_LOCK_FAIL")) { + int r = pthread_mutex_lock(&papp.thread_lock); + if(r != 0) { + printf("pthread_mutex_lock failed with status %d\n", r); + abort(); + } + } struct timespec now; clock_gettime(RTAPI_CLOCK, &now); rtapi_timespec_advance(task->nextstart, now, task->period + task->pll_correction); @@ -1086,8 +1102,13 @@ int Posix::task_self() { } void Posix::wait() { - if(do_thread_lock) - pthread_mutex_unlock(&thread_lock); + if(do_thread_lock) { + int r = pthread_mutex_unlock(&thread_lock); + if(r != 0) { + printf("pthread_mutex_unlock failed with status %d\n", r); + abort(); + } + } pthread_testcancel(); struct rtapi_task *task = reinterpret_cast(pthread_getspecific(key)); rtapi_timespec_advance(task->nextstart, task->nextstart, task->period + task->pll_correction); @@ -1103,8 +1124,13 @@ void Posix::wait() { int res = rtapi_clock_nanosleep(RTAPI_CLOCK, TIMER_ABSTIME, &task->nextstart, nullptr, &now); if(res < 0) perror("clock_nanosleep"); } - if(do_thread_lock) - pthread_mutex_lock(&thread_lock); + if(do_thread_lock) { + int r = pthread_mutex_lock(&thread_lock); + if(r != 0) { + printf("pthread_mutex_lock failed with status %d\n", r); + abort(); + } + } } unsigned char Posix::do_inb(unsigned int port)