mouse@LARRY.MCRCIM.MCGILL.EDU (der Mouse) (03/08/90)
X Window System Bug Report
xbugs@expo.lcs.mit.edu
VERSION:
R4
CLIENT MACHINE and OPERATING SYSTEM:
any
DISPLAY TYPE:
any
WINDOW MANAGER:
any
AREA:
server
SYNOPSIS:
Under certain circumstances, PlayReleasedEvents can free a
QdEventRec that is still being pointed to.
DESCRIPTION:
The queue of events waiting for a freeze to be thawed has a tail
pointer, which is used for two things: (1) to append quickly to the
tail of the queue and (2) to collapse consecutive MotionNotify
events. However, PlayReleasedEvents doesn't check carefully
enough, and if syncEvents.pendtail points into the next-to-last
event, and PlayReleasedEvents replays the next-to-last event but
not the last, syncEvents.pendtail will be left pointing into a
freed block of memory.
REPEAT BY:
The simplest thing is to examine the code. It is possible to
create this condition from clients, but I do not know precisely
what does it. I suspect that changing focus while the keyboard is
frozen may do it.
SAMPLE FIX:
The patch given below changes the meaning of syncEvents.pendtail to
be nothing but a pointer to the last pointer in the list, thus
simplifying the code to maintain and use it (and making it more
understandable thereby). A new field, syncEvents.tail, is created
to serve the other function, allowing MotionNotify event collapsing.
This patch also includes two enhancements to os/4.2bsd/utils.c:
- If gcc 1.37.1 is used to build the server on a Sun-3 running
release 3.5 (and possibly other compilers or platforms) with
MEMBUG defined, MemoryValidate is defined but it goes in the bss
segment, making it impossible to use adb to set it in the
executable file. The patch initializes it to zero, which (at
least under the above combination) puts it in the data segment
instead, where it can be patched with adb.
- When MEMBUG is defined, the entire contents of any block passed
to Xfree will be overwritten with 'Z' bytes. This greatly
increases the probability of catching a use-after-free bug (such
as the main one in this report) early.
*** /@apollo/u3/X11R4/mit/server/dix/events.c Wed Jan 24 10:17:52 1990
--- mit/server/dix/events.c Wed Mar 7 12:46:44 1990
***************
*** 90,95 ****
--- 90,96 ----
static struct {
QdEventPtr pending, *pendtail;
+ QdEventPtr tail;
DeviceIntPtr replayDev; /* kludgy rock to put flag for */
WindowPtr replayWin; /* ComputeFreezes */
Bool playingEvents;
***************
*** 482,488 ****
DeviceIntPtr device;
int count;
{
- register QdEventPtr tail = *syncEvents.pendtail;
register QdEventPtr qe;
xEvent *qxE;
--- 483,488 ----
***************
*** 492,503 ****
sprite.hotPhys.x = xE->u.keyButtonPointer.rootX;
sprite.hotPhys.y = xE->u.keyButtonPointer.rootY;
/* do motion compression */
! if (tail &&
! (tail->event->u.u.type == MotionNotify) &&
! (tail->pScreen == sprite.hotPhys.pScreen))
{
! tail->event->u.keyButtonPointer.rootX = sprite.hotPhys.x;
! tail->event->u.keyButtonPointer.rootY = sprite.hotPhys.y;
return;
}
}
--- 492,503 ----
sprite.hotPhys.x = xE->u.keyButtonPointer.rootX;
sprite.hotPhys.y = xE->u.keyButtonPointer.rootY;
/* do motion compression */
! if (syncEvents.tail &&
! (syncEvents.tail->event->u.u.type == MotionNotify) &&
! (syncEvents.tail->pScreen == sprite.hotPhys.pScreen))
{
! syncEvents.tail->event->u.keyButtonPointer.rootX = sprite.hotPhys.x;
! syncEvents.tail->event->u.keyButtonPointer.rootY = sprite.hotPhys.y;
return;
}
}
***************
*** 512,520 ****
qe->evcount = count;
for (qxE = qe->event; --count >= 0; qxE++, xE++)
*qxE = *xE;
- if (tail)
- syncEvents.pendtail = &tail->next;
*syncEvents.pendtail = qe;
}
static void
--- 512,520 ----
qe->evcount = count;
for (qxE = qe->event; --count >= 0; qxE++, xE++)
*qxE = *xE;
*syncEvents.pendtail = qe;
+ syncEvents.tail = qe;
+ syncEvents.pendtail = &qe->next;
}
static void
***************
*** 529,535 ****
--- 529,540 ----
{
*prev = qe->next;
if (!qe->next)
+ {
syncEvents.pendtail = prev;
+ syncEvents.tail = 0;
+ }
+ if (qe == syncEvents.tail)
+ FatalError("PlayReleasedEvents: tail pointer wrong");
if (qe->event->u.u.type == MotionNotify)
CheckVirtualMotion(qe, NullWindow);
syncEvents.time.months = qe->months;
***************
*** 2883,2888 ****
--- 2888,2895 ----
sprite.hotLimits.x2 = 0;
sprite.hotLimits.y2 = 0;
syncEvents.replayDev = (DeviceIntPtr)NULL;
+ syncEvents.pendtail = &syncEvents.pending;
+ syncEvents.tail = 0;
while (syncEvents.pending)
{
QdEventPtr next = syncEvents.pending->next;
***************
*** 2889,2895 ****
xfree(syncEvents.pending);
syncEvents.pending = next;
}
- syncEvents.pendtail = &syncEvents.pending;
syncEvents.playingEvents = FALSE;
currentTime.months = 0;
currentTime.milliseconds = GetTimeInMillis();
--- 2896,2901 ----
*** /@apollo/u3/X11R4/mit/server/os/4.2bsd/utils.c Thu Jan 11 13:12:40 1990
--- mit/server/os/4.2bsd/utils.c Wed Mar 7 13:06:35 1990
***************
*** 431,437 ****
unsigned long MemoryAllocTime;
unsigned long MemoryAllocBreakpoint = ~0;
unsigned long MemoryActive = 0;
! unsigned long MemoryValidate;
MallocHeaderPtr MemoryInUse;
--- 431,437 ----
unsigned long MemoryAllocTime;
unsigned long MemoryAllocBreakpoint = ~0;
unsigned long MemoryActive = 0;
! unsigned long MemoryValidate = 0;
MallocHeaderPtr MemoryInUse;
***************
*** 604,612 ****
--- 604,615 ----
if (ptr)
{
MallocHeaderPtr head;
+ register char *cp;
+ register int n;
CheckNode(ptr);
head = Header(ptr);
+ for (cp=(char *)ptr,n=(int)head->amount;n>0;n--,cp++) *cp = 'Z';
head->magic = FREEDMAGIC;
free ((char *) head);
}
der Mouse
old: mcgill-vision!mouse
new: mouse@larry.mcrcim.mcgill.edu