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 32432 invoked from network); 12 Sep 2022 14:57:50 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 12 Sep 2022 14:57:50 -0000 Received: (qmail 19871 invoked by uid 550); 12 Sep 2022 14:57:48 -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 19821 invoked from network); 12 Sep 2022 14:57:47 -0000 MIME-Version: 1.0 Date: Mon, 12 Sep 2022 17:57:35 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com In-Reply-To: <20220912142231.GJ9709@brightrain.aerifal.cx> References: <20220908091856.194976-1-izbyshev@ispras.ru> <20220912142231.GJ9709@brightrain.aerifal.cx> User-Agent: Roundcube Webmail/1.4.4 Message-ID: X-Sender: izbyshev@ispras.ru Content-Type: multipart/mixed; boundary="=_61b462a8ad4c3e03685d60ed6a1b025e" Subject: Re: [musl] [PATCH] fix thread leak on timer_create(SIGEV_THREAD) failure --=_61b462a8ad4c3e03685d60ed6a1b025e Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII; format=flowed On 2022-09-12 17:22, Rich Felker wrote: > 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. I'm not terribly happy too, but short of pulling the real pthread_cancel() here, the only other reasonable fix that I could come up with was to make the timer thread allocate the space for the deleted flag on its stack, like in the attached patch, but it felt a bit heavy. Alexey --=_61b462a8ad4c3e03685d60ed6a1b025e Content-Transfer-Encoding: base64 Content-Type: text/x-diff; name=timer-create-thread-leak.patch Content-Disposition: attachment; filename=timer-create-thread-leak.patch; size=2064 ZGlmZiAtLWdpdCBhL3NyYy90aW1lL3RpbWVyX2NyZWF0ZS5jIGIvc3JjL3RpbWUvdGltZXJfY3Jl YXRlLmMKaW5kZXggNGJlZjIzOTAuLjExN2U1ZWM2IDEwMDY0NAotLS0gYS9zcmMvdGltZS90aW1l cl9jcmVhdGUuYworKysgYi9zcmMvdGltZS90aW1lcl9jcmVhdGUuYwpAQCAtMTEsOSArMTEsMTUg QEAgc3RydWN0IGtzaWdldmVudCB7CiAJaW50IHNpZ2V2X3RpZDsKIH07CiAKK3N0cnVjdCB3b3Jr ZXIgeworCXBpZF90IHRpZDsKKwl2b2xhdGlsZSBpbnQgZGVsZXRlZDsKK307CisKIHN0cnVjdCBz dGFydF9hcmdzIHsKIAlwdGhyZWFkX2JhcnJpZXJfdCBiOwogCXN0cnVjdCBzaWdldmVudCAqc2V2 OworCXN0cnVjdCB3b3JrZXIgKnc7CiB9OwogCiBzdGF0aWMgdm9pZCBkdW1teV8wKCkKQEAgLTM4 LDExICs0NCwxNSBAQCBzdGF0aWMgdm9pZCAqc3RhcnQodm9pZCAqYXJnKQogCXB0aHJlYWRfdCBz ZWxmID0gX19wdGhyZWFkX3NlbGYoKTsKIAlzdHJ1Y3Qgc3RhcnRfYXJncyAqYXJncyA9IGFyZzsK IAlqbXBfYnVmIGpiOworCXN0cnVjdCB3b3JrZXIgdyA9IHsgc2VsZi0+dGlkIH07CiAKIAl2b2lk ICgqbm90aWZ5KSh1bmlvbiBzaWd2YWwpID0gYXJncy0+c2V2LT5zaWdldl9ub3RpZnlfZnVuY3Rp b247CiAJdW5pb24gc2lndmFsIHZhbCA9IGFyZ3MtPnNldi0+c2lnZXZfdmFsdWU7CisJYXJncy0+ dyA9ICZ3OwogCiAJcHRocmVhZF9iYXJyaWVyX3dhaXQoJmFyZ3MtPmIpOworCWlmIChzZWxmLT50 aW1lcl9pZCA8IDApCisJCXJldHVybiAwOwogCWZvciAoOzspIHsKIAkJc2lnaW5mb190IHNpOwog CQl3aGlsZSAoc2lnd2FpdGluZm8oU0lHVElNRVJfU0VULCAmc2kpIDwgMCk7CkBAIC01MSw5ICs2 MSw5IEBAIHN0YXRpYyB2b2lkICpzdGFydCh2b2lkICphcmcpCiAJCQlub3RpZnkodmFsKTsKIAkJ CXB0aHJlYWRfY2xlYW51cF9wb3AoMSk7CiAJCX0KLQkJaWYgKHNlbGYtPnRpbWVyX2lkIDwgMCkg YnJlYWs7CisJCWlmICh3LmRlbGV0ZWQpIGJyZWFrOwogCX0KLQlfX3N5c2NhbGwoU1lTX3RpbWVy X2RlbGV0ZSwgc2VsZi0+dGltZXJfaWQgJiBJTlRfTUFYKTsKKwlfX3N5c2NhbGwoU1lTX3RpbWVy X2RlbGV0ZSwgc2VsZi0+dGltZXJfaWQpOwogCXJldHVybiAwOwogfQogCkBAIC0xMTgsNyArMTI4 LDcgQEAgaW50IHRpbWVyX2NyZWF0ZShjbG9ja2lkX3QgY2xrLCBzdHJ1Y3Qgc2lnZXZlbnQgKnJl c3RyaWN0IGV2cCwgdGltZXJfdCAqcmVzdHJpY3QKIAkJdGQtPnRpbWVyX2lkID0gdGltZXJpZDsK IAkJcHRocmVhZF9iYXJyaWVyX3dhaXQoJmFyZ3MuYik7CiAJCWlmICh0aW1lcmlkIDwgMCkgcmV0 dXJuIC0xOwotCQkqcmVzID0gKHZvaWQgKikoSU5UUFRSX01JTiB8ICh1aW50cHRyX3QpdGQ+PjEp OworCQkqcmVzID0gKHZvaWQgKikoSU5UUFRSX01JTiB8ICh1aW50cHRyX3QpYXJncy53Pj4xKTsK IAkJYnJlYWs7CiAJZGVmYXVsdDoKIAkJZXJybm8gPSBFSU5WQUw7CmRpZmYgLS1naXQgYS9zcmMv dGltZS90aW1lcl9kZWxldGUuYyBiL3NyYy90aW1lL3RpbWVyX2RlbGV0ZS5jCmluZGV4IGIwYmZh YzA5Li44MGJhYTkyMCAxMDA2NDQKLS0tIGEvc3JjL3RpbWUvdGltZXJfZGVsZXRlLmMKKysrIGIv c3JjL3RpbWUvdGltZXJfZGVsZXRlLmMKQEAgLTUsOSArNSwxMiBAQAogaW50IHRpbWVyX2RlbGV0 ZSh0aW1lcl90IHQpCiB7CiAJaWYgKChpbnRwdHJfdCl0IDwgMCkgewotCQlwdGhyZWFkX3QgdGQg PSAodm9pZCAqKSgodWludHB0cl90KXQgPDwgMSk7Ci0JCWFfc3RvcmUoJnRkLT50aW1lcl9pZCwg dGQtPnRpbWVyX2lkIHwgSU5UX01JTik7Ci0JCV9fc3lzY2FsbChTWVNfdGtpbGwsIHRkLT50aWQs IFNJR1RJTUVSKTsKKwkJc3RydWN0IHdvcmtlciB7CisJCQlwaWRfdCB0aWQ7CisJCQl2b2xhdGls ZSBpbnQgZGVsZXRlZDsKKwkJfSAqdyA9ICh2b2lkICopKCh1aW50cHRyX3QpdCA8PCAxKTsKKwkJ YV9zdG9yZSgmdy0+ZGVsZXRlZCwgMSk7CisJCV9fc3lzY2FsbChTWVNfdGtpbGwsIHctPnRpZCwg U0lHVElNRVIpOwogCQlyZXR1cm4gMDsKIAl9CiAJcmV0dXJuIF9fc3lzY2FsbChTWVNfdGltZXJf ZGVsZXRlLCB0KTsK --=_61b462a8ad4c3e03685d60ed6a1b025e--