Hi Peter, we received the following report against Debian's X server package, is this something you've seen before and/or does the patch Steven provided look sane? Thanks, Julien On Sun, Apr 13, 2014 at 03:26:48 +1000, Steven McDonald wrote: > Package: xserver-xorg-core > Version: 2:1.15.0.901-1 > Severity: important > Tags: upstream patch > > Hi! > > I've been seeing sporadic (anywhere from once every few days to 3-4 > times a day) crashes and freezes in X. The problematic behaviour isn't > always the same, but I chose a particular incident to debug, and found > that X was segfaulting in updateMotionHistory, on line 575 of > dix/getevents.c. > > After some further investigation, I found that the bug was being > triggered when a SIGIO was received in DeepCopyPointerClasses, between > the AllocValuatorClass call (line 540) and updating the to->valuator > pointer (line 545). AllocValuatorClass calls realloc() on to->valuator, > so between these lines, it's not guaranteed to point to allocated > memory. > > It seems the SIGIO handler is calling updateMotionHistory, which is > reading the memory pointed to by to->valuator and getting a wrong value > for last_motion, which updates buff to point to wildly the wrong place > and thus generates a segfault when a memcpy() is done into buff. > > This diagnosis was performed on my work machine, which is the only > place I have been able to reproduce the problem, so I don't have a > backtrace or a relevant Xorg.0.log handy. Let me know if you'd like > that information, and I can supply it on Monday. > > I am attaching a patch which I've been running on that machine for the > past three days, and haven't yet observed any more crashing or freezing > behaviour. The patch simply calls OsBlockSIGIO while > DeepCopyDeviceClasses is in progress, as the state of the X server's > device data structures is not guaranteed to be in a consistent state > during that time. > > Please let me know if you need any further information. > > Thanks, > Steven. > Block SIGIOs while copying device classes around, so that we don't try > to do anything with input when devices are in an inconsistent state. > --- a/Xi/exevents.c > +++ b/Xi/exevents.c > @@ -661,6 +661,8 @@ > DeepCopyDeviceClasses(DeviceIntPtr from, DeviceIntPtr to, > DeviceChangedEvent *dce) > { > + OsBlockSIGIO(); > + > /* generic feedback classes, not tied to pointer and/or keyboard */ > DeepCopyFeedbackClasses(from, to); > > @@ -668,6 +670,8 @@ > DeepCopyKeyboardClasses(from, to); > if ((dce->flags & DEVCHANGE_POINTER_EVENT)) > DeepCopyPointerClasses(from, to); > + > + OsReleaseSIGIO(); > } > > /**
Attachment:
signature.asc
Description: Digital signature