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

diff --git a/lib/libc/thread/synch.h b/lib/libc/thread/synch.h
index c417822..2946c1b 100644 (file)
--- 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 516a97a..4f971ef 100644 (file)
--- 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) {