fix multiple bugs in SIGEV_THREAD timers
authorRich Felker <dalias@aerifal.cx>
Sat, 3 Aug 2013 17:20:42 +0000 (13:20 -0400)
committerRich Felker <dalias@aerifal.cx>
Sat, 3 Aug 2013 17:20:42 +0000 (13:20 -0400)
1. the thread result field was reused for storing a kernel timer id,
but would be overwritten if the application code exited or cancelled
the thread.

2. low pointer values were used as the indicator that the timer id is
a kernel timer id rather than a thread id. this is not portable, as
mmap may return low pointers on some conditions. instead, use the fact
that pointers must be aligned and kernel timer ids must be
non-negative to map pointers into the negative integer space.

3. signals were not blocked until after the timer thread started, so a
race condition could allow a signal handler to run in the timer thread
when it's not supposed to exist. this is mainly problematic if the
calling thread was the only thread where the signal was unblocked and
the signal handler assumes it runs in that thread.

src/internal/pthread_impl.h
src/time/timer_create.c
src/time/timer_delete.c
src/time/timer_getoverrun.c
src/time/timer_gettime.c
src/time/timer_settime.c

index 67b0575345eef1b186e6aa7de8734e6a3e5d4fe9..2e910b3e640529c50b8777fd9b366fad92f8521a 100644 (file)
@@ -38,7 +38,7 @@ struct pthread {
                void *pending;
        } robust_list;
        int unblock_cancel;
-       int delete_timer;
+       int timer_id;
        locale_t locale;
        int killlock[2];
        int exitlock[2];
index f76b9ef8f3e64412621996b020b0dd2b72670543..2b4619d4ac0738af94d5aceaddcd2afdd1664e3d 100644 (file)
@@ -60,20 +60,18 @@ static void *start(void *arg)
 {
        pthread_t self = __pthread_self();
        struct start_args *args = arg;
-       sigset_t set;
+       int id;
 
        /* Reuse no-longer-needed thread structure fields to avoid
         * needing the timer address in the signal handler. */
        self->start = (void *(*)(void *))args->sev->sigev_notify_function;
        self->start_arg = args->sev->sigev_value.sival_ptr;
-       self->result = (void *)-1;
-
-       sigfillset(&set);
-       pthread_sigmask(SIG_BLOCK, &set, 0);
 
        pthread_barrier_wait(&args->b);
-       __wait(&self->delete_timer, 0, 0, 1);
-       __syscall(SYS_timer_delete, self->result);
+       if ((id = self->timer_id) >= 0) {
+               __wait(&self->timer_id, 0, id, 1);
+               __syscall(SYS_timer_delete, id);
+       }
        return 0;
 }
 
@@ -86,6 +84,7 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict
        struct start_args args;
        struct ksigevent ksev, *ksevp=0;
        int timerid;
+       sigset_t set;
 
        switch (evp ? evp->sigev_notify : SIGEV_SIGNAL) {
        case SIGEV_NONE:
@@ -110,23 +109,25 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict
                pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
                pthread_barrier_init(&args.b, 0, 2);
                args.sev = evp;
+
+               __block_app_sigs(&set);
                r = pthread_create(&td, &attr, start, &args);
+               __restore_sigs(&set);
                if (r) {
                        errno = r;
                        return -1;
                }
+
                ksev.sigev_value.sival_ptr = 0;
                ksev.sigev_signo = SIGTIMER;
                ksev.sigev_notify = 4; /* SIGEV_THREAD_ID */
                ksev.sigev_tid = td->tid;
-               r = syscall(SYS_timer_create, clk, &ksev, &timerid);
+               if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0)
+                       timerid = -1;
+               td->timer_id = timerid;
                pthread_barrier_wait(&args.b);
-               if (r < 0) {
-                       pthread_cancel(td);
-                       return -1;
-               }
-               td->result = (void *)(intptr_t)timerid;
-               *res = td;
+               if (timerid < 0) return -1;
+               *res = (void *)(INTPTR_MIN | (uintptr_t)td>>1);
                break;
        default:
                errno = EINVAL;
index b5f8ca196a753ee890f3197797555b56098edd54..c81f921a4164ca0c795f15da014e9393c1673f9c 100644 (file)
@@ -1,12 +1,13 @@
 #include <time.h>
+#include <limits.h>
 #include "pthread_impl.h"
 
 int timer_delete(timer_t t)
 {
-       if ((uintptr_t)t >= 0x100000) {
-               pthread_t td = t;
-               td->delete_timer = 1;
-               __wake(&td->delete_timer, 1, 1);
+       if ((intptr_t)t < 0) {
+               pthread_t td = (void *)((uintptr_t)t << 1);
+               a_store(&td->timer_id, td->timer_id | INT_MIN);
+               __wake(&td->timer_id, 1, 1);
                return 0;
        }
        return __syscall(SYS_timer_delete, (long)t);
index 8a86833d4a481a9f9dbb19ccba9e110dfad2aca9..53361285f3ee028d223cabb3cb08037d1b89ec7d 100644 (file)
@@ -1,8 +1,12 @@
 #include <time.h>
+#include <limits.h>
 #include "pthread_impl.h"
 
 int timer_getoverrun(timer_t t)
 {
-       if ((uintptr_t)t >= 0x100000) t = ((pthread_t)t)->result;
+       if ((intptr_t)t < 0) {
+               pthread_t td = (void *)((uintptr_t)t << 1);
+               t = (void *)(uintptr_t)(td->timer_id & INT_MAX);
+       }
        return syscall(SYS_timer_getoverrun, (long)t);
 }
index 5a85821894cfa57b6728ee207e3a90d2a9c3a1b7..1d902075c4354e52fd54dd2bf3fd75029c9c5676 100644 (file)
@@ -1,8 +1,12 @@
 #include <time.h>
+#include <limits.h>
 #include "pthread_impl.h"
 
 int timer_gettime(timer_t t, struct itimerspec *val)
 {
-       if ((uintptr_t)t >= 0x100000) t = ((pthread_t)t)->result;
+       if ((intptr_t)t < 0) {
+               pthread_t td = (void *)((uintptr_t)t << 1);
+               t = (void *)(uintptr_t)(td->timer_id & INT_MAX);
+       }
        return syscall(SYS_timer_gettime, (long)t, val);
 }
index baf5076b6c3f979aa18b00468f896ef766f57a07..f5f36feb2896c013362e197715b56f7969d5f800 100644 (file)
@@ -1,8 +1,12 @@
 #include <time.h>
+#include <limits.h>
 #include "pthread_impl.h"
 
 int timer_settime(timer_t t, int flags, const struct itimerspec *restrict val, struct itimerspec *restrict old)
 {
-       if ((uintptr_t)t >= 0x100000) t = ((pthread_t)t)->result;
+       if ((intptr_t)t < 0) {
+               pthread_t td = (void *)((uintptr_t)t << 1);
+               t = (void *)(uintptr_t)(td->timer_id & INT_MAX);
+       }
        return syscall(SYS_timer_settime, (long)t, flags, val, old);
 }