From d13c8b7725437490be8c1a2b438936af10f808d0 Mon Sep 17 00:00:00 2001 From: Pauli Date: Wed, 27 Jul 2022 11:52:17 +1000 Subject: [PATCH] Make OSSL_TIME a structure This prevents misuses creeping in. Reviewed-by: Hugo Landau Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/18882) --- doc/internal/man3/OSSL_TIME.pod | 40 ++++++++++++---- include/internal/time.h | 83 +++++++++++++++++++++------------ ssl/event_queue.c | 6 +-- ssl/quic/quic_wire.c | 12 +++-- ssl/time.c | 15 +++--- test/event_queue_test.c | 37 +++++++++------ test/quic_wire_test.c | 5 +- 7 files changed, 130 insertions(+), 68 deletions(-) diff --git a/doc/internal/man3/OSSL_TIME.pod b/doc/internal/man3/OSSL_TIME.pod index c7091d0832..81dad2e0d9 100644 --- a/doc/internal/man3/OSSL_TIME.pod +++ b/doc/internal/man3/OSSL_TIME.pod @@ -2,7 +2,8 @@ =head1 NAME -OSSL_TIME, OSSL_TIME_SECOND, OSSL_TIME_INFINITY, +OSSL_TIME, OSSL_TIME_SECOND, ossl_time_infinite, ossl_time_zero, +ossl_ticks2time, ossl_time2ticks, ossl_time_now, ossl_time_time_to_timeval, ossl_time_compare, ossl_time_add, ossl_time_subtract - times and durations @@ -11,12 +12,17 @@ ossl_time_add, ossl_time_subtract #include "internal/time.h" - typedef uint64_t OSSL_TIME; + typedef struct OSSL_TIME; - #define OSSL_TIME_SECOND - #define OSSL_TIME_INFINITY + #define OSSL_TIME_SECOND /* Ticks per second */ + OSSL_TIME ossl_ticks2time(uint64_t); + uint64_t ossl_time2ticks(OSSL_TIME t); + + OSSL_TIME ossl_time_zero(void); + OSSL_TIME ossl_time_infinite(void); OSSL_TIME ossl_time_now(void); + void ossl_time_time_to_timeval(OSSL_TIME t, struct timeval *out); int ossl_time_compare(OSSL_TIME a, OSSL_TIME b); @@ -40,8 +46,16 @@ B. Specifically, it is the number of counts per second that a time can represent. The accuracy is independent of this and is system dependent. -B is the largest representable B. This value -is returned when an overflow would otherwise occur. +B converts an integral number of counts to a time. + +B converts a time to an integral number of counts. + +B returns the smallest representable B. +This value represents the time Epoch and it is returned when an underflow +would otherwise occur. + +B returns the largest representable B. +This value is returned when an overflow would otherwise occur. B returns the current time relative to an Epoch which is undefined but unchanging for at least the duration of program @@ -70,15 +84,23 @@ The largest representable duration is guaranteed to be at least 500 years. =head1 RETURN VALUES -B returns the current time, or 0 on error. +B returns the current time, or the time of the Epoch on error. + +B returns the time of the Epoch. + +B returns the last representable time. + +B return the duration specified. + +B returns the ticks since Epoch. B returns -1, 0 or 1 depending on the comparison. B returns the summation of the two times or -B on overflow. +the last representable time on overflow. B returns the difference of the two times or the -0 on underflow. +time of the Epoch on underflow. =head1 HISTORY diff --git a/include/internal/time.h b/include/internal/time.h index b4a67fb998..50f50822a8 100644 --- a/include/internal/time.h +++ b/include/internal/time.h @@ -15,6 +15,17 @@ # include "internal/e_os.h" /* for struct timeval */ # include "internal/safe_math.h" +/* + * Internal type defining a time. + * This should be treated as an opaque structure. + * + * The time datum is Unix's 1970 and at nanosecond precision, this gives + * a range of 584 years roughly. + */ +typedef struct { + uint64_t t; /* Ticks since the epoch */ +} OSSL_TIME; + /* The precision of times allows this many values per second */ # define OSSL_TIME_SECOND ((uint64_t)1000000000) @@ -24,44 +35,55 @@ /* One microsecond. */ # define OSSL_TIME_US (OSSL_TIME_MS / 1000) -/* Macro representing the most distant future time */ -# define OSSL_TIME_INFINITY (~(OSSL_TIME)0) +/* Convert a tick count into a time */ +static ossl_unused ossl_inline OSSL_TIME ossl_ticks2time(uint64_t ticks) +{ + OSSL_TIME r; -/* Macro that's guaranteed to be now or before */ -# define OSSL_TIME_IMMEDIATE 0 + r.t = ticks; + return r; +} -/* Macro representing the zero value */ -# define OSSL_TIME_ZERO 0 - -/* - * Internal type defining a time. - * The time datum is Unix's 1970 and at nanosecond precision, this gives - * a range of 584 years roughly. - */ -typedef uint64_t OSSL_TIME; +/* Convert a time to a tick count */ +static ossl_unused ossl_inline uint64_t ossl_time2ticks(OSSL_TIME t) +{ + return t.t; +} /* Get current time */ OSSL_TIME ossl_time_now(void); +/* The beginning and end of the time range */ +static ossl_unused ossl_inline OSSL_TIME ossl_time_zero(void) +{ + return ossl_ticks2time(0); +} + +static ossl_unused ossl_inline OSSL_TIME ossl_time_infinite(void) +{ + return ossl_ticks2time(~(uint64_t)0); +} + + /* Convert time to timeval */ static ossl_unused ossl_inline void ossl_time_time_to_timeval(OSSL_TIME t, struct timeval *out) { #ifdef _WIN32 - out->tv_sec = (long int)(t / OSSL_TIME_SECOND); + out->tv_sec = (long int)(t.t / OSSL_TIME_SECOND); #else - out->tv_sec = (time_t)(t / OSSL_TIME_SECOND); + out->tv_sec = (time_t)(t.t / OSSL_TIME_SECOND); #endif - out->tv_usec = (t % OSSL_TIME_SECOND) / (OSSL_TIME_SECOND / 1000000); + out->tv_usec = (t.t % OSSL_TIME_SECOND) / (OSSL_TIME_SECOND / 1000000); } /* Compare two time values, return -1 if less, 1 if greater and 0 if equal */ static ossl_unused ossl_inline int ossl_time_compare(OSSL_TIME a, OSSL_TIME b) { - if (a > b) + if (a.t > b.t) return 1; - if (a < b) + if (a.t < b.t) return -1; return 0; } @@ -71,7 +93,7 @@ int ossl_time_compare(OSSL_TIME a, OSSL_TIME b) * These operations are saturating, in that an overflow or underflow returns * the largest or smallest value respectively. */ -OSSL_SAFE_MATH_UNSIGNED(time, OSSL_TIME) +OSSL_SAFE_MATH_UNSIGNED(time, uint64_t) static ossl_unused ossl_inline OSSL_TIME ossl_time_add(OSSL_TIME a, OSSL_TIME b) @@ -79,8 +101,8 @@ OSSL_TIME ossl_time_add(OSSL_TIME a, OSSL_TIME b) OSSL_TIME r; int err = 0; - r = safe_add_time(a, b, &err); - return err ? OSSL_TIME_INFINITY : r; + r.t = safe_add_time(a.t, b.t, &err); + return err ? ossl_time_infinite() : r; } static ossl_unused ossl_inline @@ -89,15 +111,16 @@ OSSL_TIME ossl_time_subtract(OSSL_TIME a, OSSL_TIME b) OSSL_TIME r; int err = 0; - r = safe_sub_time(a, b, &err); - return err ? 0 : r; + r.t = safe_sub_time(a.t, b.t, &err); + return err ? ossl_time_zero() : r; } /* Returns |a - b|. */ static ossl_unused ossl_inline OSSL_TIME ossl_time_abs_difference(OSSL_TIME a, OSSL_TIME b) { - return a > b ? ossl_time_subtract(a, b) : ossl_time_subtract(b, a); + return a.t > b.t ? ossl_time_subtract(a, b) + : ossl_time_subtract(b, a); } static ossl_unused ossl_inline @@ -106,8 +129,8 @@ OSSL_TIME ossl_time_multiply(OSSL_TIME a, uint64_t b) OSSL_TIME r; int err = 0; - r = safe_mul_time(a, b, &err); - return err ? OSSL_TIME_INFINITY : r; + r.t = safe_mul_time(a.t, b, &err); + return err ? ossl_time_infinite() : r; } static ossl_unused ossl_inline @@ -116,22 +139,22 @@ OSSL_TIME ossl_time_divide(OSSL_TIME a, uint64_t b) OSSL_TIME r; int err = 0; - r = safe_div_time(a, b, &err); - return err ? 0 : r; + r.t = safe_div_time(a.t, b, &err); + return err ? ossl_time_zero() : r; } /* Return higher of the two given time values. */ static ossl_unused ossl_inline OSSL_TIME ossl_time_max(OSSL_TIME a, OSSL_TIME b) { - return a > b ? a : b; + return a.t > b.t ? a : b; } /* Return the lower of the two given time values. */ static ossl_unused ossl_inline OSSL_TIME ossl_time_min(OSSL_TIME a, OSSL_TIME b) { - return a < b ? a : b; + return a.t < b.t ? a : b; } #endif diff --git a/ssl/event_queue.c b/ssl/event_queue.c index f828d5280f..ed3c790904 100644 --- a/ssl/event_queue.c +++ b/ssl/event_queue.c @@ -145,16 +145,16 @@ int ossl_event_queue_remove(OSSL_EVENT_QUEUE *queue, OSSL_EVENT *event) OSSL_TIME ossl_event_time_until(const OSSL_EVENT *event) { if (event == NULL) - return OSSL_TIME_INFINITY; + return ossl_time_infinite(); return ossl_time_subtract(event->when, ossl_time_now()); } OSSL_TIME ossl_event_queue_time_until_next(const OSSL_EVENT_QUEUE *queue) { if (queue == NULL) - return OSSL_TIME_INFINITY; + return ossl_time_infinite(); if (ossl_pqueue_OSSL_EVENT_num(queue->now_events) > 0) - return OSSL_TIME_IMMEDIATE; + return ossl_time_zero(); return ossl_event_time_until(ossl_pqueue_OSSL_EVENT_peek(queue->timed_events)); } diff --git a/ssl/quic/quic_wire.c b/ssl/quic/quic_wire.c index 0adb41ef59..a3a9b252fa 100644 --- a/ssl/quic/quic_wire.c +++ b/ssl/quic/quic_wire.c @@ -48,13 +48,15 @@ int ossl_quic_wire_encode_frame_ack(WPACKET *pkt, uint64_t largest_ackd, first_ack_range, ack_delay_enc; size_t i, num_ack_ranges = ack->num_ack_ranges; + OSSL_TIME delay; if (num_ack_ranges == 0) return 0; - ack_delay_enc = ossl_time_divide(ossl_time_divide(ack->delay_time, - OSSL_TIME_US), - 1UL << ack_delay_exponent); + delay = ossl_time_divide(ossl_time_divide(ack->delay_time, OSSL_TIME_US), + 1UL << ack_delay_exponent); + ack_delay_enc = ossl_time2ticks(delay); + largest_ackd = ack->ack_ranges[0].end; first_ack_range = ack->ack_ranges[0].end - ack->ack_ranges[0].start; @@ -443,12 +445,12 @@ int ossl_quic_wire_decode_frame_ack(PACKET *pkt, if (ack != NULL) { int err = 0; ack->delay_time - = ossl_time_multiply(OSSL_TIME_US, + = ossl_time_multiply(ossl_ticks2time(OSSL_TIME_US), safe_mul_uint64_t(ack_delay_raw, 1UL << ack_delay_exponent, &err)); if (err) - ack->delay_time = OSSL_TIME_INFINITY; + ack->delay_time = ossl_time_infinite(); if (ack->num_ack_ranges > 0) { ack->ack_ranges[0].end = largest_ackd; diff --git a/ssl/time.c b/ssl/time.c index ecdd6ea8cb..a1b9dc7534 100644 --- a/ssl/time.c +++ b/ssl/time.c @@ -13,6 +13,8 @@ OSSL_TIME ossl_time_now(void) { + OSSL_TIME r; + #if defined(_WIN32) SYSTEMTIME st; union { @@ -28,19 +30,20 @@ OSSL_TIME ossl_time_now(void) # else now.ul -= 116444736000000000UI64; # endif - return ((uint64_t)now.ul) * (OSSL_TIME_SECOND / 10000000); + r.t = ((uint64_t)now.ul) * (OSSL_TIME_SECOND / 10000000); #else struct timeval t; if (gettimeofday(&t, NULL) < 0) { ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(), "calling gettimeofday()"); - return 0; + return ossl_time_zero(); } if (t.tv_sec <= 0) - return t.tv_usec <= 0 ? 0 : t.tv_usec * (OSSL_TIME_SECOND / 1000000); - return ((uint64_t)t.tv_sec * 1000000 + t.tv_usec) - * (OSSL_TIME_SECOND / 1000000); + r.t = t.tv_usec <= 0 ? 0 : t.tv_usec * (OSSL_TIME_SECOND / 1000000); + else + r.t = ((uint64_t)t.tv_sec * 1000000 + t.tv_usec) + * (OSSL_TIME_SECOND / 1000000); #endif + return r; } - diff --git a/test/event_queue_test.c b/test/event_queue_test.c index 76765c5ba1..686233c930 100644 --- a/test/event_queue_test.c +++ b/test/event_queue_test.c @@ -11,7 +11,7 @@ #include "internal/nelem.h" #include "testutil.h" -static OSSL_TIME cur_time = 100; +static OSSL_TIME cur_time = { 100 }; OSSL_TIME ossl_time_now(void) { @@ -31,38 +31,49 @@ static int event_test(void) /* Create an event queue and add some events */ if (!TEST_ptr(q = ossl_event_queue_new()) - || !TEST_ptr(e1 = ossl_event_queue_add_new(q, 1, 10, 1100, "ctx 1", + || !TEST_ptr(e1 = ossl_event_queue_add_new(q, 1, 10, + ossl_ticks2time(1100), + "ctx 1", PAYLOAD(payload))) - || !TEST_ptr(e2 = ossl_event_queue_add_new(q, 2, 5, 1100, "ctx 2", + || !TEST_ptr(e2 = ossl_event_queue_add_new(q, 2, 5, + ossl_ticks2time(1100), + "ctx 2", PAYLOAD("data"))) - || !TEST_true(ossl_event_queue_add(q, &e3, 3, 20, 1200, "ctx 3", + || !TEST_true(ossl_event_queue_add(q, &e3, 3, 20, + ossl_ticks2time(1200), "ctx 3", PAYLOAD("more data"))) - || !TEST_ptr(e4 = ossl_event_queue_add_new(q, 2, 5, 1150, "ctx 2", + || !TEST_ptr(e4 = ossl_event_queue_add_new(q, 2, 5, + ossl_ticks2time(1150), + "ctx 2", PAYLOAD("data"))) /* Verify some event details */ || !TEST_uint_eq(ossl_event_get_type(e1), 1) || !TEST_uint_eq(ossl_event_get_priority(e1), 10) - || !TEST_uint64_t_eq(ossl_event_get_when(e1), 1100) + || !TEST_uint64_t_eq(ossl_time2ticks(ossl_event_get_when(e1)) + , 1100) || !TEST_str_eq(ossl_event_get0_ctx(e1), "ctx 1") || !TEST_ptr(p = ossl_event_get0_payload(e1, &len)) || !TEST_str_eq((char *)p, payload) - || !TEST_uint64_t_eq(ossl_event_time_until(&e3), 1100) - || !TEST_uint64_t_eq(ossl_event_queue_time_until_next(q), 1000) + || !TEST_uint64_t_eq(ossl_time2ticks(ossl_event_time_until(&e3)), + 1100) + || !TEST_uint64_t_eq(ossl_time2ticks(ossl_event_queue_time_until_next(q)), + 1000) /* Modify an event's time */ - || !TEST_true(ossl_event_queue_postpone_until(q, e1, 1200)) - || !TEST_uint64_t_eq(ossl_event_get_when(e1), 1200) + || !TEST_true(ossl_event_queue_postpone_until(q, e1, + ossl_ticks2time(1200))) + || !TEST_uint64_t_eq(ossl_time2ticks(ossl_event_get_when(e1)), 1200) || !TEST_true(ossl_event_queue_remove(q, e4))) goto err; ossl_event_free(e4); /* Execute the queue */ - cur_time = 1000; + cur_time = ossl_ticks2time(1000); if (!TEST_true(ossl_event_queue_get1_next_event(q, &ep)) || !TEST_ptr_null(ep)) goto err; - cur_time = 1100; + cur_time = ossl_ticks2time(1100); if (!TEST_true(ossl_event_queue_get1_next_event(q, &ep)) || !TEST_ptr_eq(ep, e2)) goto err; @@ -72,7 +83,7 @@ static int event_test(void) || !TEST_ptr_null(ep)) goto err; - cur_time = 1250; + cur_time = ossl_ticks2time(1250); if (!TEST_true(ossl_event_queue_get1_next_event(q, &ep)) || !TEST_ptr_eq(ep, &e3)) goto err; diff --git a/test/quic_wire_test.c b/test/quic_wire_test.c index 55d18aa27c..10c7835e0c 100644 --- a/test/quic_wire_test.c +++ b/test/quic_wire_test.c @@ -80,7 +80,7 @@ static const OSSL_QUIC_ACK_RANGE encode_case_3_ranges[] = { static const OSSL_QUIC_FRAME_ACK encode_case_3_f = { (OSSL_QUIC_ACK_RANGE *)encode_case_3_ranges, OSSL_NELEM(encode_case_3_ranges), - OSSL_TIME_MS, + { OSSL_TIME_MS }, 60, 70, 80, 1 }; @@ -123,7 +123,8 @@ static int encode_case_3_dec(PACKET *pkt, ossl_ssize_t fail) encode_case_3_f.num_ack_ranges * sizeof(OSSL_QUIC_ACK_RANGE))) return 0; - if (!TEST_uint64_t_eq(f.delay_time, encode_case_3_f.delay_time)) + if (!TEST_uint64_t_eq(ossl_time2ticks(f.delay_time), + ossl_time2ticks(encode_case_3_f.delay_time))) return 0; if (!TEST_true(f.ecn_present))