[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#744303: xserver-xorg-core: Segmentation fault when receiving a SIGIO in DeepCopyDeviceClasses



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


Reply to: