From 2e686a7fd2561c81dde123d8a2bdf2f23b1bce4b Mon Sep 17 00:00:00 2001 From: cheloha Date: Sun, 7 Jan 2024 19:44:28 +0000 Subject: [PATCH] libc, librthread: _twait: subtraction is not comparison Compare the current time with the absolute timeout before computing the relative timeout to avoid arithmetic overflow. Fixes a bug where large negative absolute timeouts are subtracted into large positive relative timeouts and incorrectly cause the caller to block. While here, use timespeccmp(3) and timespecsub(3) to simplify the code. Thread: https://marc.info/?l=openbsd-tech&m=169945962503129&w=2 --- lib/libc/thread/synch.h | 14 +++++--------- lib/librthread/synch.h | 14 +++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/libc/thread/synch.h b/lib/libc/thread/synch.h index c417822bb43..2946c1be319 100644 --- a/lib/libc/thread/synch.h +++ b/lib/libc/thread/synch.h @@ -1,4 +1,4 @@ -/* $OpenBSD: synch.h,v 1.8 2023/11/08 15:51:28 cheloha Exp $ */ +/* $OpenBSD: synch.h,v 1.9 2024/01/07 19:44:28 cheloha Exp $ */ /* * Copyright (c) 2017 Martin Pieuchot * @@ -28,7 +28,7 @@ _wake(volatile uint32_t *p, int n) static inline int _twait(volatile uint32_t *p, int val, clockid_t clockid, const struct timespec *abs) { - struct timespec rel; + struct timespec now, rel; int saved_errno = errno; int error; @@ -41,16 +41,12 @@ _twait(volatile uint32_t *p, int val, clockid_t clockid, const struct timespec * return error; } - if (!timespecisvalid(abs) || WRAP(clock_gettime)(clockid, &rel)) + if (!timespecisvalid(abs) || WRAP(clock_gettime)(clockid, &now)) return EINVAL; - rel.tv_sec = abs->tv_sec - rel.tv_sec; - if ((rel.tv_nsec = abs->tv_nsec - rel.tv_nsec) < 0) { - rel.tv_sec--; - rel.tv_nsec += 1000000000; - } - if (rel.tv_sec < 0) + if (timespeccmp(abs, &now, <=)) return ETIMEDOUT; + timespecsub(abs, &now, &rel); error = futex(p, FUTEX_WAIT_PRIVATE, val, &rel, NULL); if (error == -1) { diff --git a/lib/librthread/synch.h b/lib/librthread/synch.h index 516a97afa64..4f971ef2ec2 100644 --- a/lib/librthread/synch.h +++ b/lib/librthread/synch.h @@ -1,4 +1,4 @@ -/* $OpenBSD: synch.h,v 1.9 2023/11/08 15:51:28 cheloha Exp $ */ +/* $OpenBSD: synch.h,v 1.10 2024/01/07 19:44:28 cheloha Exp $ */ /* * Copyright (c) 2017 Martin Pieuchot * @@ -28,7 +28,7 @@ _wake(volatile uint32_t *p, int n) static inline int _twait(volatile uint32_t *p, int val, clockid_t clockid, const struct timespec *abs) { - struct timespec rel; + struct timespec now, rel; int saved_errno = errno; int error; @@ -41,16 +41,12 @@ _twait(volatile uint32_t *p, int val, clockid_t clockid, const struct timespec * return error; } - if (!timespecisvalid(abs) || clock_gettime(clockid, &rel)) + if (!timespecisvalid(abs) || clock_gettime(clockid, &now)) return EINVAL; - rel.tv_sec = abs->tv_sec - rel.tv_sec; - if ((rel.tv_nsec = abs->tv_nsec - rel.tv_nsec) < 0) { - rel.tv_sec--; - rel.tv_nsec += 1000000000; - } - if (rel.tv_sec < 0) + if (timespeccmp(abs, &now, <=)) return ETIMEDOUT; + timespecsub(abs, &now, &rel); error = futex(p, FUTEX_WAIT, val, &rel, NULL); if (error == -1) { -- 2.20.1