complib/timer: fix race accessing timer fields

The timeout_time field of cl_timer_t is modified in the timer
callback function and also in calls to cl_timer_start, cl_timer_stop,
and cl_timer_trim.  The user cannot protect against the changes
made in the timer callback function, so the timer must provide
this protection itself.

Signed-off-by: Sean Hefty <sean.hefty@intel.com>

diff --git a/trunk/core/complib/kernel/cl_timer.c b/trunk/core/complib/kernel/cl_timer.c
index 34cae8e3caa70a4526f9576623f678f65f6e136b..ccbc79440bfc1d8bdb26ed302ad5f6f61e56c08d 100644 (file)
--- a/trunk/core/complib/kernel/cl_timer.c
+++ b/trunk/core/complib/kernel/cl_timer.c
@@ -45,7 +45,9 @@ __timer_callback(
        UNUSED_PARAM( arg1 );\r
        UNUSED_PARAM( arg2 );\r
 \r
+       cl_spinlock_acquire( &p_timer->spinlock );\r
        p_timer->timeout_time = 0;\r
+       cl_spinlock_release( &p_timer->spinlock );\r
 \r
        (p_timer->pfn_callback)( (void*)p_timer->context );\r
 }\r
@@ -75,6 +77,7 @@ cl_timer_init(
 \r
        KeInitializeTimer( &p_timer->timer );\r
        KeInitializeDpc( &p_timer->dpc, __timer_callback, p_timer );\r
+       cl_spinlock_init( &p_timer->spinlock );\r
 \r
        return( CL_SUCCESS );\r
 }\r
@@ -108,6 +111,7 @@ cl_timer_start(
        IN      const uint32_t          time_ms )\r
 {\r
        LARGE_INTEGER   due_time;\r
+       uint64_t                timeout_time;\r
 \r
        CL_ASSERT( p_timer );\r
        CL_ASSERT( p_timer->pfn_callback );\r
@@ -115,11 +119,13 @@ cl_timer_start(
 \r
        /* Due time is in 100 ns increments.  Negative for relative time. */\r
        due_time.QuadPart = -(int64_t)(((uint64_t)time_ms) * 10000);\r
+       timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
 \r
-       /* Store the timeout time in the timer object. */\r
-       p_timer->timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
-\r
+       cl_spinlock_acquire( &p_timer->spinlock );\r
+       p_timer->timeout_time = timeout_time;\r
        KeSetTimer( &p_timer->timer, due_time, &p_timer->dpc );\r
+       cl_spinlock_release( &p_timer->spinlock );\r
+\r
        return( CL_SUCCESS );\r
 }\r
 \r
@@ -129,19 +135,25 @@ cl_timer_trim(
        IN      cl_timer_t* const       p_timer,\r
        IN      const uint32_t          time_ms )\r
 {\r
+       LARGE_INTEGER   due_time;\r
        uint64_t                timeout_time;\r
 \r
        CL_ASSERT( p_timer );\r
        CL_ASSERT( p_timer->pfn_callback );\r
 \r
-       /* Calculate the timeout time in the timer object. */\r
+       /* Due time is in 100 ns increments.  Negative for relative time. */\r
+       due_time.QuadPart = -(int64_t)(((uint64_t)time_ms) * 10000);\r
        timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
 \r
        /* Only pull in the timeout time. */\r
-       if( p_timer->timeout_time && p_timer->timeout_time < timeout_time )\r
-               return( CL_SUCCESS );\r
-\r
-       return cl_timer_start( p_timer, time_ms );\r
+       cl_spinlock_acquire( &p_timer->spinlock );\r
+       if( !p_timer->timeout_time || p_timer->timeout_time > timeout_time )\r
+       {\r
+               p_timer->timeout_time = timeout_time;\r
+               KeSetTimer( &p_timer->timer, due_time, &p_timer->dpc );\r
+       }\r
+       cl_spinlock_release( &p_timer->spinlock );\r
+       return( CL_SUCCESS );\r
 }\r
 \r
 \r
@@ -154,7 +166,8 @@ cl_timer_stop(
        CL_ASSERT( KeGetCurrentIrql() <= DISPATCH_LEVEL );\r
        \r
        /* Cancel the timer.  This also cancels any queued DPCs for the timer. */\r
-       KeCancelTimer( &p_timer->timer );\r
-\r
+       cl_spinlock_acquire( &p_timer->spinlock );\r
        p_timer->timeout_time = 0;\r
+       KeCancelTimer( &p_timer->timer );\r
+       cl_spinlock_release( &p_timer->spinlock );\r
 }\r

diff --git a/trunk/inc/kernel/complib/cl_timer_osd.h b/trunk/inc/kernel/complib/cl_timer_osd.h
index 4cb99b0831d884b3ead73e45c0f4d2b629b1928e..32fef834124c6df8f67f697d522243e9371b20f7 100644 (file)
--- a/trunk/inc/kernel/complib/cl_timer_osd.h
+++ b/trunk/inc/kernel/complib/cl_timer_osd.h
@@ -38,7 +38,7 @@
 \r
 \r
 #include "complib/cl_types.h"\r
-\r
+#include "complib/cl_spinlock.h"\r
 \r
 /* Timer object definition. */\r
 typedef struct _cl_timer\r
@@ -48,6 +48,7 @@ typedef struct _cl_timer
        cl_pfn_timer_callback_t pfn_callback;\r
        const void                              *context;\r
        uint64_t                                timeout_time;\r
+       cl_spinlock_t                   spinlock;\r
 \r
 } cl_timer_t;\r
 \r