[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260903232718.GQ3542221@port70.net>
Date: Fri, 4 Sep 2026 01:27:18 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: Colin Cross <ccross@...roid.com>
Cc: musl@...ts.openwall.com
Subject: Re: Fix deadlock in timer_create when syscall fails
* Colin Cross <ccross@...roid.com> [2026-09-03 10:26:12 -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.
this looks ok, i have minor comments only
your orig mail had
This was observed in a test that called
timer_create(CLOCK_BOOTTIME_ALARM, SIGEV_THREAD, ...) without
CAP_WAKE_ALARM, causing the kernel to return EPERM.
i prefer to include such details in the commit.
> ---
> src/time/timer_create.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> index cc6c2236..a89f45af 100644
> --- a/src/time/timer_create.c
> +++ b/src/time/timer_create.c
> @@ -45,19 +45,28 @@ static void *start(void *arg)
> pthread_t self = __pthread_self();
> struct start_args *args = arg;
> jmp_buf jb;
> + int cs;
not needed.
>
> 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;
or
int timerid;
while (sem_wait(...));
timerid = self->timer_id; // only one if this way
sem_post(...);
if (timerid < 0) return 0;
> for (;;) {
> siginfo_t si;
> @@ -135,7 +144,6 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict
> ksev.sigev_tid = td->tid;
> if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0) {
> timerid = -1;
> - td->cancel = 1;
> }
> td->timer_id = timerid;
> sem_post(&args.sem1);
> --
> 2.55.0.979.g7e5102b832-goog
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
