From 803d17b4c04b1b078bffa431eb717fc1de919ee7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 2 Sep 2026 13:38:08 -0700 Subject: [PATCH] SIGEV_THREAD timers: fix deadlock when timer_create syscall fails sem_wait is a cancellation point. If the parent thread in timer_create sets td->cancel = 1 before the child thread reaches sem_wait then the thread will exit and sem_post will never be called, causing the parent thread to deadlock in sem_wait. 3ad3fa962efee12067d68c3405a537dce156a7ac set td->cancel = 1 to fix a thread leak when the syscall failed. The switch to two-way semaphores cde213f9c3ac1aa168581222edee6a6642113323 allows checking if self->timer_id is -1 instead of self->cancel because timer_delete can't have been called before the sem_post allows the parent thread to return from timer_create. The deadlock was observed in a test that called timer_create(CLOCK_BOOTTIME_ALARM, SIGEV_THREAD, ...) without CAP_WAKE_ALARM, causing the kernel to return EPERM. Also disable cancellation around the parent semaphores to avoid the parent being cancelled while the child is still accessing the args on the parent stack, and save the result of the timer_create syscall to return as errno on error. --- src/time/timer_create.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/time/timer_create.c b/src/time/timer_create.c index cc6c2236..31861c14 100644 --- a/src/time/timer_create.c +++ b/src/time/timer_create.c @@ -48,16 +48,24 @@ static void *start(void *arg) void (*notify)(union sigval) = args->sev->sigev_notify_function; union sigval val = args->sev->sigev_value; + int cancel = 0; - /* The two-way semaphore synchronization ensures that we see - * self->cancel set by the parent if timer creation failed or - * self->timer_id if it succeeded, and informs the parent that - * we are done accessing the arguments so that the parent can - * proceed past their block lifetime. */ + /* Waiting on sem1 ensures we see self->timer_id. */ while (sem_wait(&args->sem1)); + + /* If self->timer_id is -1 while the parent thread is still in + * timer_create (before the sem_post to sem2) then the timer was + * never created. If it is -1 later after the sem_post then + * it was cancelled via timer_delete. */ + if (self->timer_id < 0) + cancel = 1; + + /* Incrementing sem2 informs the parent that we are done checking + * the initial value of self->timer_id, and accessing the arguments + * so that the parent can proceed past their block lifetime. */ sem_post(&args->sem2); - if (self->cancel) + if (cancel) return 0; for (;;) { siginfo_t si; @@ -83,6 +91,7 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict struct ksigevent ksev, *ksevp=0; int timerid; sigset_t set; + int cs; switch (evp ? evp->sigev_notify : SIGEV_SIGNAL) { case SIGEV_NONE: @@ -133,14 +142,22 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict ksev.sigev_signo = SIGTIMER; ksev.sigev_notify = SIGEV_THREAD_ID; ksev.sigev_tid = td->tid; - if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0) { + r = __syscall(SYS_timer_create, clk, &ksev, &timerid); + if (r < 0) { timerid = -1; - td->cancel = 1; } td->timer_id = timerid; + + __pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); sem_post(&args.sem1); while (sem_wait(&args.sem2)); - if (timerid < 0) return -1; + __pthread_setcancelstate(cs, 0); + + if (timerid < 0) { + errno = -r; + return -1; + } + *res = (void *)(INTPTR_MIN | (uintptr_t)td>>1); break; default: -- 2.55.0.1032.g73a4cd73de-goog