commit c1d96358d49a4583b8aa9bdb1e8d73c70f9d8d06 Author: Nick Mathewson <nickm@torproject.org> Date: Tue Jun 1 16:18:23 2021 -0400 Use native timegm when available. Continue having a tor_gmtime_impl() unit test so that we can detect any problems in our replacement function; add a new test function to make sure that gmtime<->timegm are a round-trip on now-ish times. This is a fix for bug #40383, wherein we ran into trouble because tor_timegm() does not believe that time_t should include a count of leap seconds, but FreeBSD's gmtime believes that it should. This disagreement meant that for a certain amount of time each day, instead of calculating the most recent midnight, our voting-schedule functions would calculate the second-most-recent midnight, and lead to an assertion failure. I am calling this a bugfix on 0.2.0.3-alpha when we first started calculating our voting schedule in this way. --- changes/bug40383 | 7 +++++++ configure.ac | 1 + src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++-- src/lib/encoding/time_fmt.h | 6 ++++++ src/test/test_util.c | 26 +++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/changes/bug40383 b/changes/bug40383 new file mode 100644 index 0000000000..c4ca46fac7 --- /dev/null +++ b/changes/bug40383 @@ -0,0 +1,7 @@ + o Minor bugfixes (timekeeping): + - Calculate the time of day correctly on systems where the time_t + type includes leap seconds. (This is not the case on most + operating systems, but on those where it occurs, our tor_timegm + function did not correctly invert the system's gmtime function, + which could result in assertion failures when calculating + voting schedules.) Fixes bug 40383; bugfix on 0.2.0.3-alpha. diff --git a/configure.ac b/configure.ac index 6d7cd1dd6a..5a01c28bde 100644 --- a/configure.ac +++ b/configure.ac @@ -793,6 +793,7 @@ AC_CHECK_FUNCS( strtoull \ sysconf \ sysctl \ + timegm \ truncate \ uname \ usleep \ diff --git a/src/lib/encoding/time_fmt.c b/src/lib/encoding/time_fmt.c index 573dfaad82..5e58d36698 100644 --- a/src/lib/encoding/time_fmt.c +++ b/src/lib/encoding/time_fmt.c @@ -13,6 +13,7 @@ * and handles a larger variety of types. It converts between different time * formats, and encodes and decodes them from strings. **/ +#define TIME_FMT_PRIVATE #include "lib/encoding/time_fmt.h" #include "lib/log/log.h" @@ -25,6 +26,7 @@ #include <string.h> #include <time.h> +#include <errno.h> #ifdef HAVE_SYS_TIME_H #include <sys/time.h> @@ -92,8 +94,8 @@ static const int days_per_month[] = /** Compute a time_t given a struct tm. The result is given in UTC, and * does not account for leap seconds. Return 0 on success, -1 on failure. */ -int -tor_timegm(const struct tm *tm, time_t *time_out) +ATTR_UNUSED STATIC int +tor_timegm_impl(const struct tm *tm, time_t *time_out) { /* This is a pretty ironclad timegm implementation, snarfed from Python2.2. * It's way more brute-force than fiddling with tzset(). @@ -162,6 +164,35 @@ tor_timegm(const struct tm *tm, time_t *time_out) return 0; } +/** Compute a time_t given a struct tm. The result here should be an inverse + * of the system's gmtime() function. Return 0 on success, -1 on failure. + */ +int +tor_timegm(const struct tm *tm, time_t *time_out) +{ +#ifdef HAVE_TIMEGM + /* If the system gives us a timegm(), use it: if the system's time_t + * includes leap seconds, then we can hope that its timegm() knows too. + * + * https://k5wiki.kerberos.org/wiki/Leap_second_handling says the in + * general we can rely on any system with leap seconds also having a + * timegm implementation. Let's hope it's right! + * */ + time_t result = timegm((struct tm *) tm); + if (result == -1) { + log_warn(LD_BUG, "timegm() could not convert time: %s", strerror(errno)); + *time_out = 0; + return -1; + } else { + *time_out = result; + return 0; + } +#else + /* The system doesn't have timegm; we'll have to use our own. */ + return tor_timegm_impl(tm, time_out); +#endif +} + /* strftime is locale-specific, so we need to replace those parts */ /** A c-locale array of 3-letter names of weekdays, starting with Sun. */ diff --git a/src/lib/encoding/time_fmt.h b/src/lib/encoding/time_fmt.h index 80e47c5332..4adccb5990 100644 --- a/src/lib/encoding/time_fmt.h +++ b/src/lib/encoding/time_fmt.h @@ -18,6 +18,8 @@ #include <sys/types.h> #endif +#include "lib/testsupport/testsupport.h" + struct tm; struct timeval; @@ -41,4 +43,8 @@ int parse_iso_time_nospace(const char *cp, time_t *t); int parse_http_time(const char *buf, struct tm *tm); int format_time_interval(char *out, size_t out_len, long interval); +#ifdef TIME_FMT_PRIVATE +STATIC int tor_timegm_impl(const struct tm *tm, time_t *time_out); +#endif + #endif /* !defined(TOR_TIME_FMT_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index d43bf781f2..ab63344806 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -7,6 +7,7 @@ #define COMPAT_TIME_PRIVATE #define UTIL_MALLOC_PRIVATE #define PROCESS_WIN32_PRIVATE +#define TIME_FMT_PRIVATE #include "lib/testsupport/testsupport.h" #include "core/or/or.h" #include "lib/buf/buffers.h" @@ -111,7 +112,7 @@ static time_t tor_timegm_wrapper(const struct tm *tm) { time_t t; - if (tor_timegm(tm, &t) < 0) + if (tor_timegm_impl(tm, &t) < 0) return -1; return t; } @@ -1501,6 +1502,28 @@ test_util_parse_http_time(void *arg) teardown_capture_of_logs(); } +static void +test_util_timegm_real(void *arg) +{ + (void)arg; + /* Get the real timegm again! We're not testing our impl; we want the + * one that will actually get called. */ +#undef tor_timegm + + /* Now check: is timegm the real inverse of gmtime? */ + time_t now = time(NULL), time2=0; + struct tm tm, *p; + p = tor_gmtime_r(&now, &tm); + tt_ptr_op(p, OP_NE, NULL); + + int r = tor_timegm(&tm, &time2); + tt_int_op(r, OP_EQ, 0); + tt_i64_op((int64_t) now, OP_EQ, (int64_t) time2); + + done: + ; +} + static void test_util_config_line(void *arg) { @@ -7043,6 +7066,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(monotonic_time_ratchet, TT_FORK), UTIL_TEST(monotonic_time_zero, 0), UTIL_TEST(monotonic_time_add_msec, 0), + UTIL_TEST(timegm_real, 0), UTIL_TEST(htonll, 0), UTIL_TEST(get_unquoted_path, 0), UTIL_TEST(map_anon, 0),