From fee13cf6ecf3bbc5290a7c13bc93ab1132cbaca4 Mon Sep 17 00:00:00 2001 From: Sam Hartman Date: Tue, 9 Aug 2011 20:07:34 +0000 Subject: [PATCH] Fix rare duplicate time issue On systems with imprecise clocks (e.g. windows), there was as issue where microsecond rollover could conceivably cause the same time to be reported twice. Also document potential performance improvement by using thread-local storage for last_time and eliminating the mutex. Signed-off-by: Kevin Wasserman Signed-off-by: Sam Hartman git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25088 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/os/c_ustime.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/lib/krb5/os/c_ustime.c b/src/lib/krb5/os/c_ustime.c index 3dabe801b..d374b1347 100644 --- a/src/lib/krb5/os/c_ustime.c +++ b/src/lib/krb5/os/c_ustime.c @@ -83,6 +83,11 @@ krb5_crypto_us_timeofday(krb5_int32 *seconds, krb5_int32 *microseconds) if (err) return err; + /* It would probably be more efficient to remove this mutex and use + thread-local storage for last_time. But that could result in + different threads getting the same value for time, which may be + a technical violation of spec. */ + err = k5_mutex_lock(&krb5int_us_time_mutex); if (err) return err; @@ -94,16 +99,24 @@ krb5_crypto_us_timeofday(krb5_int32 *seconds, krb5_int32 *microseconds) On Windows, where we get millisecond accuracy currently, that's quite likely. On UNIX, it appears that we always get new microsecond values, so this case should never trigger. */ - if ((now.sec == last_time.sec) && (now.usec <= last_time.usec)) { - /* Same as last time??? */ + + /* Check for case where previously usec rollover caused bump in sec, + putting now.sec in the past. But don't just use '<' because we + need to properly handle the case where the administrator intentionally + adjusted time backwards. */ + if ((now.sec == last_time.sec-1) || + ((now.sec == last_time.sec) && (now.usec <= last_time.usec))) { + /* Correct 'now' to be exactly one microsecond later than 'last_time'. + Note that _because_ we perform this hack, 'now' may be _earlier_ + than 'last_time', even though the system time is monotonically + increasing. */ + + now.sec = last_time.sec; now.usec = ++last_time.usec; if (now.usec >= 1000000) { ++now.sec; now.usec = 0; } - /* For now, we're not worrying about the case of enough - returns of the same value that we roll over now.sec, and - the next call still gets the previous now.sec value. */ } last_time.sec = now.sec; /* Remember for next time */ last_time.usec = now.usec; -- 2.26.2