From 2484ffeb81e1abe37ba55d8f465768f09fdfe732 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 16 Oct 2017 19:33:06 +0200 Subject: [PATCH] get_timer: don't use uninitialized memory 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. --- ChangeLog | 1 + library/timing.c | 45 +++++++++++++++++++++++---------------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8b11c181f..b3bab778f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -44,6 +44,7 @@ Bugfix * 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/library/timing.c b/library/timing.c index 6e7f93fe2..25acff2cd 100644 --- a/library/timing.c +++ b/library/timing.c @@ -234,21 +234,23 @@ volatile int alarmed = 0; unsigned long get_timer( struct 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 @@ void m_sleep( int milliseconds ) unsigned long get_timer( struct 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 ); + } } #if defined(INTEGRITY)