--- Begin Message ---
Package: xserver-xorg-core
Version: 2:1.1.1-21etch3
Severity: normal
Tags: patch
Hi,
The Debian patch 38_wait_for_something_force_timer_reset.diff has a
problem:
Consider the case when there is a time warp, CheckAllTimers() removes
_all_ timers and "timers" becomes NULL. Upon return from it,
"timers->expires" is referenced unconditionally in WaitForSomething().
I am 98% certain I have seen a crash from this on my system when ntp
adjusted the system time just while the X server was starting (details
available on request).
I believe the whole timeout computation logic is best moved into
CheckAllTimers (including the magic constant 250) so I append
what I believe patch 38_wait_for_something_force_timer_reset.diff
should look like.
Regards,
Wolfram
>From upstream commits 8d3b465eb3d6c93cbbcebe8e5c9298caaaeb650b and
bb7a39ac13731a80fc2d80487f9da760dd34c3ba
Patch by Daniel Stone.
Fixed timers becoming NULL, W. Gloger.
Index: xorg-server-1.1.1/os/WaitFor.c
===================================================================
--- xorg-server-1.1.1.orig/os/WaitFor.c 2006-07-05 20:38:48.000000000 +0200
+++ xorg-server-1.1.1/os/WaitFor.c 2008-02-15 21:59:11.000000000 +0100
@@ -128,11 +128,13 @@
struct _OsTimerRec {
OsTimerPtr next;
CARD32 expires;
+ CARD32 delta;
OsTimerCallback callback;
pointer arg;
};
static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev);
+static CARD32 CheckAllTimers(void);
static OsTimerPtr timers = NULL;
/*****************
@@ -205,12 +207,9 @@
{
#endif
wt = NULL;
- if (timers)
+ timeout = CheckAllTimers();
+ if (timeout >= 0)
{
- now = GetTimeInMillis();
- timeout = timers->expires - now;
- if (timeout < 0)
- timeout = 0;
waittime.tv_sec = timeout / MILLI_PER_SECOND;
waittime.tv_usec = (timeout % MILLI_PER_SECOND) *
(1000000 / MILLI_PER_SECOND);
@@ -447,6 +446,33 @@
}
#endif
+/* If time has rewound, re-run every affected timer.
+ * Timers might drop out of the list, so we have to restart every time. */
+static CARD32
+CheckAllTimers(void)
+{
+ OsTimerPtr timer;
+ CARD32 now, timeout;
+
+ if (!timers)
+ return -1;
+ now = GetTimeInMillis();
+ timeout = timers->expires - now;
+ if (timeout > 0 && timeout > timers->delta + 250) {
+ /* time has rewound. reset the timers. */
+ start:
+ for (timer = timers; timer; timer = timer->next) {
+ if (timer->expires - now > timer->delta + 250) {
+ TimerForce(timer);
+ goto start;
+ }
+ }
+ if (!timers)
+ return -1;
+ timeout = timers->expires - now;
+ }
+ return timeout < 0 ? 0 : timeout;
+}
static void
DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev)
@@ -488,8 +514,13 @@
}
if (!millis)
return timer;
- if (!(flags & TimerAbsolute))
+ if (flags & TimerAbsolute) {
+ timer->delta = millis - now;
+ }
+ else {
+ timer->delta = millis;
millis += now;
+ }
timer->expires = millis;
timer->callback = func;
timer->arg = arg;
@@ -503,7 +534,7 @@
for (prev = &timers;
*prev && (int) ((*prev)->expires - millis) <= 0;
prev = &(*prev)->next)
- ;
+ ;
timer->next = *prev;
*prev = timer;
return timer;
-- System Information:
Debian Release: 4.0
APT prefers stable
APT policy: (500, 'stable')
Architecture: amd64 (x86_64)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.23.16
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Versions of packages xserver-xorg-core depends on:
ii libc6 2.3.6.ds1-13etch4 GNU C Library: Shared libraries
ii libfontenc1 1:1.0.2-2 X11 font encoding library
ii libxau6 1:1.0.1-2 X11 authorisation library
ii libxdmcp6 1:1.0.1-2 X11 Display Manager Control Protoc
ii libxfont1 1:1.2.2-2.etch1 X11 font rasterisation library
ii x11-common 1:7.1.0-19 X Window System (X.Org) infrastruc
ii xserver-xorg 1:7.1.0-19 the X.Org X server
ii zlib1g 1:1.2.3-13 compression library - runtime
Versions of packages xserver-xorg-core recommends:
ii xfonts-base 1:1.0.0-4 standard fonts for X
ii xkb-data 0.9-4 X Keyboard Extension (XKB) configu
-- no debconf information
--- End Message ---
--- Begin Message ---
forcemerge 415627 466002
thank you
Wolfram Gloger wrote:
> Package: xserver-xorg-core
> Version: 2:1.1.1-21etch3
> Severity: normal
> Tags: patch
>
> Hi,
>
> The Debian patch 38_wait_for_something_force_timer_reset.diff has a
> problem:
>
> Consider the case when there is a time warp, CheckAllTimers() removes
> _all_ timers and "timers" becomes NULL. Upon return from it,
> "timers->expires" is referenced unconditionally in WaitForSomething().
> I am 98% certain I have seen a crash from this on my system when ntp
> adjusted the system time just while the X server was starting (details
> available on request).
>
> I believe the whole timeout computation logic is best moved into
> CheckAllTimers (including the magic constant 250) so I append
> what I believe patch 38_wait_for_something_force_timer_reset.diff
> should look like.
>
Well, Xserver 1.1 is long gone, we have 1.4 in unstable now and the above
patch has been removed from the packaging a while ago since it has been
integrated upstream. I don't think we can update Etch just for a rare crash
like the above unfortunately.
It's a known fact Xserver will crash if ntp adjust time back in the past.
I am merging with the other bugs, which have been marked as fixed in 1.3.
Brice
--- End Message ---