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 <djku...@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 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel