XV race condition with xf86XVReputImage, Expose events and Unichrome driver

[Barry Scott]

Jon Nettleton wrote:
> On Thu, 2007-05-24 at 09:43 +0200, Michel Dänzer wrote:
>   
>> On Wed, 2007-05-23 at 16:46 -0400, Jon Nettleton wrote:
>>     
>>> After some testing on this I have tracked down the issue to the commit
>>> of this patch.
>>>
>>> http://gitweb.freedesktop.org/?p=xorg/xserver.git;a=commitdiff;h=dc914ced69e1e619a944c7dcb940c25e195cd4f3
>>>
>>> Specifically these changes
>>>
>>> @@ -1739,9 +1746,13 @@ xf86XVPutImage(
>>>       REGION_UNINIT(pScreen, &VPReg)
>>>    }
>>>
>>> -  if(portPriv->pDraw) {
>>> +  /* If we are changing windows, unregister our port in the old window */
>>> +  if(portPriv->pDraw && (portPriv->pDraw != pDraw))
>>>       xf86XVRemovePortFromWindow((WindowPtr)(portPriv->pDraw), portPriv);
>>> -  }
>>> +
>>> +  /* Register our port with the new window */
>>> +  ret =  xf86XVEnlistPortInWindow((WindowPtr)pDraw, portPriv);
>>> +  if(ret != Success) goto PUT_IMAGE_BAILOUT;
>>>
>>>    if(!REGION_NOTEMPTY(pScreen, &ClipRegion)) {
>>>       clippedAway = TRUE;
>>> @@ -1772,7 +1783,6 @@ xf86XVPutImage(
>>>    if((ret == Success) &&
>>>          (portPriv->AdaptorRec->flags & VIDEO_OVERLAID_IMAGES)) {
>>>
>>> -     xf86XVEnlistPortInWindow((WindowPtr)pDraw, portPriv);
>>>       portPriv->isOn = XV_ON;
>>>       portPriv->pDraw = pDraw;
>>>       portPriv->drw_x = drw_x;  portPriv->drw_y = drw_y;
>>>
>>>
>>> I haven't had any time to fully debug where the failure is, but
>>> backing this patch out fixes the problem.
>>>       
>> Does this patch fix it?
>>
>>     
> This is the part of the patch that caused the problem.  If you back out
> this patch it allows Xvideo to work properly on VIA video chipsets (Not
> sure if it happens on other chipsets).  Alternatively you can download
> the patch I use to revert it in my rpm's
> http://www.hekanetworks.com/~jnettlet/xserver-1.3.0-xv-minimize-crash.patch
>
> Hopefully this weekend I can get some time to figure out why this patch
> has such ill effects.
>   
Maybe you are seeing the same bug that we found and reported 2 weeks ago.
We worked around this by changing the driver to not support reput.

Barry

-------- Original Message --------
Subject: 	XV race condition with xf86XVReputImage, Expose events and 
Unichrome driver
Date: 	Thu, 10 May 2007 15:34:43 +0100
From: 	Barry Scott <barry.scott at onelan.co.uk>
To: 	xorg at lists.freedesktop.org



I'm sending this report from Simon Farnsworth to the list.
I'll raise a bug report about this unless you don't want one raised.

I've been tracking down an X server crash in our system, which
appears to be triggered by bugs in xf86xv.c.

Our hardware is a VIA EPIA M10000 (CLE266 graphics), using the
driver from unichrome.sf.net, xorg-server 1.3.0, and Linux 2.6.
We have a single instance of Xine running, using the xv output driver.

When we tell xine to stop playing one movie and to start playing
another movie we see the following sequence of events:

xf86XVClipNotify is called, and the test at line 1135
succeeds as visible is set to 0. This causes pPriv->pDraw
to be set to NULL (line 1143). Trapping X here in the
debugger for a couple of minutes is sufficient to fix the bug.

If we don't stall X, the next call is to
xf86XVWindowExposures; this ends up calling
xf86XVReputImage (line 1082).

xf86XVReputImage assumes that pPriv->pDraw is not NULL,
resulting in a SIGSEGV when it dereferences it (line 871 in an optimised 
build).

If we stall X in xf86XVClipNotify for long enough, the next call
we see is to xf86XVStopVideo, which closes down Xv, ensuring
that we don't see the crash.

For our system, the workaround is to remove ReputImage support
from the device driver, which prevents the call to xf86XVReputImage,
and thus avoids the crash.
-- 
Simon Farnsworth