[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 12 Sep 2022 10:22:31 -0400
From: Rich Felker <dalias@...c.org>
To: Alexey Izbyshev <izbyshev@...ras.ru>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] fix thread leak on timer_create(SIGEV_THREAD)
failure
On Thu, Sep 08, 2022 at 12:18:56PM +0300, Alexey Izbyshev wrote:
> After commit 5b74eed3b301e2227385f3bf26d3bb7c2d822cf8 the timer thread
> doesn't check whether timer_create() actually created the timer,
> proceeding to wait for a signal that might never arrive. We can't fix
> this by simply checking for a negative timer_id after
> pthread_barrier_wait() because we have no way to distinguish a timer
> creation failure and a request to delete a timer with INT_MAX id if it
> happens to arrive quickly (a variation of this bug existed before
> 5b74eed3b301e2227385f3bf26d3bb7c2d822cf8, where the timer would be
> leaked in this case). So (ab)use cancel field of pthread_t instead.
> ---
> src/time/timer_create.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> index 4bef2390..cd32c945 100644
> --- a/src/time/timer_create.c
> +++ b/src/time/timer_create.c
> @@ -43,6 +43,8 @@ static void *start(void *arg)
> union sigval val = args->sev->sigev_value;
>
> pthread_barrier_wait(&args->b);
> + if (self->cancel)
> + return 0;
> for (;;) {
> siginfo_t si;
> while (sigwaitinfo(SIGTIMER_SET, &si) < 0);
> @@ -113,8 +115,10 @@ 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)
> + if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0) {
> timerid = -1;
> + td->cancel = 1;
> + }
> td->timer_id = timerid;
> pthread_barrier_wait(&args.b);
> if (timerid < 0) return -1;
> --
> 2.37.2
I'm not really happy with overloading td->cancel like this, but it's
probably the best fix at present. The long-term direction for this
functionality is hopefully removing the use of kernel timer objects
entirely for SIGEV_THREAD timers and instead implementing them with
clock_nanosleep, which would eliminate the possibility of
SYS_timer_create failure, the possibility of leak, the td->timer_id
member, the SIGTIMER reserved signal, and basically all the hacks
going on here.
Rich
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.
