Bug 44354 - Crash in Mach64UploadToScreen on ATI 3D Rage II+
Summary: Crash in Mach64UploadToScreen on ATI 3D Rage II+
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/mach64 (show other bugs)
Version: 6.9.0
Hardware: PowerPC Linux (All)
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on: 51137
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-31 14:21 UTC by thor
Modified: 2013-01-05 01:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description thor 2011-12-31 14:21:37 UTC
Turning on the EXA acceleration for the mach64 driver on the ATI 3D Rage II+ creates a segfault as soon as anything serious (for example xterm or gdm) opens on the X server. Note that the mach64 driver does not support DRI for the card.

The segfault appears in Mach64UploadToScreen() in atimach64exa.c, to be specific in the memcpy(). Debugging reveals that pDst->devPrivate.ptr of the Pixmap passed in is NULL and hence the driver crashes. Playing with various options in the Device section of the X server does not change the situation, except turning off acceleration or changing the acceleration mode from EXA to XAA. Note that the card is slow in any event, but any type of acceleration would be welcome.

While it is unclear to me why libexa passes in a NULL Pixmap destination pointer to the call, the following patch seems to fix or work around the problem:

    char  *dst        = pDst->devPrivate.ptr;
+   if (dst == NULL)
+        return FALSE;
    int    dst_pitch  = exaGetPixmapPitch(pDst);

    int bpp    = pDst->drawable.bitsPerPixel;
    int cpp    = (bpp + 7) / 8;
    int wBytes = w * cpp;

A similar patch should also be applied to Mach64DownloadFromScreen() for similar reasons. Probably the bug is not in the mach64 driver but in libexa or any other component of X not passing in valid pointer in first place.
Comment 1 thor 2012-01-01 16:52:55 UTC
The same bug and the same patch also applies to the 6.9.0 release, no difference there.
Comment 2 Michel Dänzer 2012-01-03 04:33:22 UTC
(In reply to comment #2)
> Probably the bug is not in the mach64 driver but in libexa or
> any other component of X not passing in valid pointer in first place.

No, that's intentional. ->devPrivate.ptr is only valid between PrepareAccess and FinishAccess for any pixmap.

This means that your patch effectively makes Mach64UploadToScreen and Mach64DownloadFromScreen no-ops. I don't think they served any purpose anyway and could just be dropped. Not sure what the comment below was referring to, but I don't think it's been true for a long time.

    /* EXA hits more optimized paths when it does not have to fallback because
     * of missing UTS/DFS, hook memcpy-based UTS/DFS.
     */
Comment 3 thor 2012-01-03 04:59:28 UTC
It might very well be that devPrivate.ptr is only known to be valid under specific circumstances. However, quite apparently, as the code *does crash*, these circumstances are not met when the call is made, at least not always.

This then rather points in the direction that libexa has a bug, namely calling the mentioned function in cases where such conditions are not met.

The code, as it is now, cannot stay at least as it makes exa use impossible for machII+ owners. While I'm not advocating this patch in specific, it would at least allow to use the optimized path in *some* situations, and it would fall back to safe use in all others. 

That is, it does not make the call *always* a no-op, but only in cases the pointer is not valid. If that is indeed always the case (as you suggest, but I can neither confirm nor deny it), then the UploadToScreen function (and friends) should really be removed as they just crash whenever used.



The correct solution would be, of course, to locate the bug in libexa in first place, agreed.
Comment 4 thor 2012-06-16 15:36:10 UTC
Any update on this bug? Have the affected code parts been replaced or patched?
Comment 5 Theodore Waddell 2013-01-05 00:34:39 UTC
Is there any update on this? I am using 6.9.1 on Debian Wheezy PPC with a Rage Pro LT and am experiencing X segfaults whenever EXA is specificed in the xorg.conf.
Comment 6 Theodore Waddell 2013-01-05 01:28:33 UTC
I compiled & installed the 6.9.4 mach64 driver and my log confirmed it was in use, but the issue persists.
Comment 7 Alan Coopersmith 2013-01-05 01:33:25 UTC
This issue was fixed in xf86-video-mach64 6.9.4 which deleted
Mach64UploadToScreen and Mach64DownloadFromScreen and uses the
server-provided defaults instead.

Crashes in 6.9.4 would not be this bug (which is "crash in Mach64UploadToScreen").


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.