[PATCH 1/3] os: block signals when accessing global timer list

[Daniel Kurtz]

X Input drivers, such as xf86-input-synaptics, tend to do all of their
processing in a SIGIO signal handler.  This processing often involves
creating, modifying or canceling a timer.  Any of these operations may
modify the global "timers" array.  Therefore, all accesses of this global
must be done in critical secitions during which signals are blocked.

Otherwise, for example, a signal may clear the last timer between, which
sets timers global to NULL, between the NULL check and checking "expires",
which causes a SEGV.

A previous patch protected write accesses.  However, this is not
sufficient. Read accesses must also be protected from a signal
occurring between when the timers is NULL checked and subsequent
dereferences.

The redundant signal blocking in DoTimer() and CheckAllTimers() is removed
since they are always called with signals already blocked.

Also, make the global |timers| volatile to ensure that the compiler does
not cache its value.

Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
---
 os/WaitFor.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/os/WaitFor.c b/os/WaitFor.c
index 393890f..8630fc9 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -122,7 +122,7 @@ struct _OsTimerRec {
 
 static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev);
 static void CheckAllTimers(void);
-static OsTimerPtr timers = NULL;
+volatile static OsTimerPtr timers = NULL;
 
 /*****************
  * WaitForSomething:
@@ -186,6 +186,7 @@ WaitForSomething(int *pClientsReady)
         }
         else {
             wt = NULL;
+            OsBlockSignals();
             if (timers) {
                 now = GetTimeInMillis();
                 timeout = timers->expires - now;
@@ -204,6 +205,7 @@ WaitForSomething(int *pClientsReady)
                     wt = &waittime;
                 }
             }
+            OsReleaseSignals();
             XFD_COPYSET(&AllSockets, &LastSelectMask);
         }
 
@@ -251,6 +253,7 @@ WaitForSomething(int *pClientsReady)
             if (*checkForInput[0] != *checkForInput[1])
                 return 0;
 
+            OsBlockSignals();
             if (timers) {
                 int expired = 0;
 
@@ -261,14 +264,18 @@ WaitForSomething(int *pClientsReady)
                 while (timers && (int) (timers->expires - now) <= 0)
                     DoTimer(timers, now, &timers);
 
-                if (expired)
+                if (expired) {
+                    OsReleaseSignals();
                     return 0;
+                }
             }
+            OsReleaseSignals();
         }
         else {
             fd_set tmp_set;
 
             if (*checkForInput[0] == *checkForInput[1]) {
+                OsBlockSignals();
                 if (timers) {
                     int expired = 0;
 
@@ -279,9 +286,12 @@ WaitForSomething(int *pClientsReady)
                     while (timers && (int) (timers->expires - now) <= 0)
                         DoTimer(timers, now, &timers);
 
-                    if (expired)
+                    if (expired) {
+                        OsReleaseSignals();
                         return 0;
+                    }
                 }
+                OsReleaseSignals();
             }
             if (someReady)
                 XFD_ORSET(&LastSelectMask, &ClientsWithInput, &LastSelectMask);
@@ -382,7 +392,6 @@ CheckAllTimers(void)
     OsTimerPtr timer;
     CARD32 now;
 
-    OsBlockSignals();
  start:
     now = GetTimeInMillis();
 
@@ -392,7 +401,6 @@ CheckAllTimers(void)
             goto start;
         }
     }
-    OsReleaseSignals();
 }
 
 static void
@@ -400,13 +408,11 @@ DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev)
 {
     CARD32 newTime;
 
-    OsBlockSignals();
     *prev = timer->next;
     timer->next = NULL;
     newTime = (*timer->callback) (timer, now, timer->arg);
     if (newTime)
         TimerSet(timer, 0, newTime, timer->callback, timer->arg);
-    OsReleaseSignals();
 }
 
 OsTimerPtr
@@ -508,10 +514,13 @@ TimerFree(OsTimerPtr timer)
 void
 TimerCheck(void)
 {
-    CARD32 now = GetTimeInMillis();
+    CARD32 now;
 
+    OsBlockSignals();
+    now = GetTimeInMillis();
     while (timers && (int) (timers->expires - now) <= 0)
         DoTimer(timers, now, &timers);
+    OsReleaseSignals();
 }
 
 void
@@ -519,10 +528,12 @@ TimerInit(void)
 {
     OsTimerPtr timer;
 
+    OsBlockSignals();
     while ((timer = timers)) {
         timers = timer->next;
         free(timer);
     }
+    OsReleaseSignals();
 }
 
 #ifdef DPMSExtension
-- 
1.7.7.3