mbedtls_timing_get_timer: don't use uninitialized memory
mbedtls_timing_get_timer with reset=1 is called both to initialize a
timer object and to reset an already-initialized object. In an
initial call, the content of the data structure is indeterminate, so
the code should not read from it. This could crash if signed overflows
trap, for example.
As a consequence, on reset, we can't return the previously elapsed
time as was previously done on Windows. Return 0 as was done on Unix.
diff --git a/ChangeLog b/ChangeLog
index 5c3eaa5..76d4b54 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -54,6 +54,7 @@
* Fix crash when calling mbedtls_ssl_cache_free() twice. Found by
MilenkoMitrovic, #1104
* Fix mbedtls_timing_alarm(0) on Unix.
+ * Fix use of uninitialized memory in mbedtls_timing_get_timer when reset=1.
Changes
* Extend cert_write example program by options to set the CRT version
diff --git a/include/mbedtls/timing.h b/include/mbedtls/timing.h
index 579de33..bfb8579 100644
--- a/include/mbedtls/timing.h
+++ b/include/mbedtls/timing.h
@@ -75,9 +75,18 @@
* \brief Return the elapsed time in milliseconds
*
* \param val points to a timer structure
- * \param reset if set to 1, the timer is restarted
+ * \param reset If 0, query the elapsed time. Otherwise (re)start the timer.
*
- * \return Elapsed time in ms (before the reset, if there is a reset)
+ * \return Elapsed time since the previous reset in ms. When
+ * restarting, this is always 0.
+ *
+ * \note To initialize a timer, call this function with reset=1.
+ *
+ * Determining the elapsed time and resetting the timer is not
+ * atomic on all platforms, so after the sequence
+ * `{ get_timer(1); ...; time1 = get_timer(1); ...; time2 =
+ * get_timer(0) }` the value time1+time2 is only approximately
+ * the delay since the first reset.
*/
unsigned long mbedtls_timing_get_timer( struct mbedtls_timing_hr_time *val, int reset );
diff --git a/library/timing.c b/library/timing.c
index 4ec4bf4..af0d6cc 100644
--- a/library/timing.c
+++ b/library/timing.c
@@ -239,21 +239,23 @@
unsigned long mbedtls_timing_get_timer( struct mbedtls_timing_hr_time *val, int reset )
{
- unsigned long delta;
- LARGE_INTEGER offset, hfreq;
struct _hr_time *t = (struct _hr_time *) val;
- QueryPerformanceCounter( &offset );
- QueryPerformanceFrequency( &hfreq );
-
- delta = (unsigned long)( ( 1000 *
- ( offset.QuadPart - t->start.QuadPart ) ) /
- hfreq.QuadPart );
-
if( reset )
+ {
QueryPerformanceCounter( &t->start );
-
- return( delta );
+ return( 0 );
+ }
+ else
+ {
+ unsigned long delta;
+ LARGE_INTEGER now, hfreq;
+ QueryPerformanceCounter( &now );
+ QueryPerformanceFrequency( &hfreq );
+ delta = (unsigned long)( ( now.QuadPart - t->start.QuadPart ) * 1000ul
+ / hfreq.QuadPart );
+ return( delta );
+ }
}
/* It's OK to use a global because alarm() is supposed to be global anyway */
@@ -280,23 +282,22 @@
unsigned long mbedtls_timing_get_timer( struct mbedtls_timing_hr_time *val, int reset )
{
- unsigned long delta;
- struct timeval offset;
struct _hr_time *t = (struct _hr_time *) val;
- gettimeofday( &offset, NULL );
-
if( reset )
{
- t->start.tv_sec = offset.tv_sec;
- t->start.tv_usec = offset.tv_usec;
+ gettimeofday( &t->start, NULL );
return( 0 );
}
-
- delta = ( offset.tv_sec - t->start.tv_sec ) * 1000
- + ( offset.tv_usec - t->start.tv_usec ) / 1000;
-
- return( delta );
+ else
+ {
+ unsigned long delta;
+ struct timeval now;
+ gettimeofday( &now, NULL );
+ delta = ( now.tv_sec - t->start.tv_sec ) * 1000ul
+ + ( now.tv_usec - t->start.tv_usec ) / 1000;
+ return( delta );
+ }
}
static void sighandler( int signum )