From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 28015 invoked from network); 12 Sep 2022 14:22:47 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 12 Sep 2022 14:22:47 -0000 Received: (qmail 24473 invoked by uid 550); 12 Sep 2022 14:22:44 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 24418 invoked from network); 12 Sep 2022 14:22:43 -0000 Date: Mon, 12 Sep 2022 10:22:31 -0400 From: Rich Felker To: Alexey Izbyshev Cc: musl@lists.openwall.com Message-ID: <20220912142231.GJ9709@brightrain.aerifal.cx> References: <20220908091856.194976-1-izbyshev@ispras.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220908091856.194976-1-izbyshev@ispras.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [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