Bug 23298

I can reproduce on git master versions of xorg-server & libpixman from today.

Building debuggable libpixman shows that pixman is striding out of the 
shared memory pixmap:

t@1 (l@1) signal SEGV (no mapping at the fault address) in pixman_blt_mmx at line 3054 in file "pixman-mmx.c"
 3054   
(dbx) where
current thread: t@1
=>[1] pixman_blt_mmx(src_bits = 0xfd2d3000, dst_bits = 0xf9200000, src_stride = 2400, dst_stride = 6400, src_bpp = 32, dst_bpp = 32, src_x = 574, src_y = 504, dst_x = 577, dst_y = 507, width = 10, height = 9), line 3054 in "pixman-mmx.c"
  [2] mmx_blt(imp = 0x83b2e50, src_bits = 0xfd2d3000, dst_bits = 0xf9200000, src_stride = 600, dst_stride = 1600, src_bpp = 32, dst_bpp = 32, src_x = 574, src_y = 504, dst_x = 577, dst_y = 507, width = 10, height = 10), line 3347 in "pixman-mmx.c"
  [3] _pixman_implementation_blt(imp = 0x83b2e50, src_bits = 0xfd2d3000, dst_bits = 0xf9200000, src_stride = 600, dst_stride = 1600, src_bpp = 32, dst_bpp = 32, src_x = 574, src_y = 504, dst_x = 577, dst_y = 507, width = 10, height = 10), line 250 in "pixman-implementation.c"
  [4] pixman_blt(src_bits = 0xfd2d3000, dst_bits = 0xf9200000, src_stride = 600, dst_stride = 1600, src_bpp = 32, dst_bpp = 32, src_x = 574, src_y = 504, dst_x = 577, dst_y = 507, width = 10, height = 10), line 236 in "pixman.c"
  [5] fbCopyNtoN(0x86eddd8, 0x86eb740, 0x86ed918, 0x8047230, 0x1, 0xfffffffd, 0xfffffffd, 0x0, 0x0, 0x0, 0x0, 0x8275edd), at 0xfe57d6c9 
  [6] miDoCopy(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8, 0xfe57d33c, 0x0, 0x0, 0x0), at 0x82768dc 
  [7] fbCopyArea(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8, 0xfe558534, 0x80472f8, 0xfe4efa4e), at 0xfe57dfcc 
  [8] XAACopyAreaFallback(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8, 0xfe55853c, 0x8047358, 0xfe4f0e68), at 0xfe4efaaf 
  [9] XAACopyArea(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8, 0x80473c8, 0x80473cc, 0xfe5430ce), at 0xfe4f1164 
  [10] cwCopyArea(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8, 0x83303b4, 0x0, 0x81b3bbd), at 0xfe5431b0 
  [11] damageCopyArea(0x86eddd8, 0x86eb740, 0x86ed918, 0x23e, 0x1f8, 0xa, 0xa, 0x23e, 0x1f8), at 0x81b4376 
  [12] ProcShmPutImage(0x86e92d8, 0x0, 0x0, 0x818a8b1), at 0x8189f89 
  [13] ProcShmDispatch(0x86e92d8, 0x8a, 0x80477c8, 0x809aad2), at 0x818ad5f 
  [14] Dispatch(0x0, 0x83048a0, 0x0, 0x83331b0, 0x83331b4, 0x8332ad0), at 0x809ab83 
  [15] main(0x4, 0x80478e4, 0x80478f8, 0x807f36f), at 0x80d8ddc 
(dbx) print d
d = 0xf9518c04 "ÿÿÿ"
(dbx) print s
s = 0xfd3fadf8 "<bad address 0xfd3fadf8>"
(dbx) print * (char *) 0xfd2d3000 
*((char *) 0xfd2d3000U) = '\0'
(dbx) pmap -xF 1911
1911:   /usr/X11R7/bin/Xorg :0 -config /etc/X11/xorg.conf-1.7
 Address  Kbytes     RSS    Anon  Locked Mode   Mapped File
[....]
FD2D3000    1072    1072       -       - r-xs-    [ shmid=0x8 ]
FD3E0000      16      16       -       - r-x--  kbd_drv.so
FD3F3000      12      12       4       - rwx--  kbd_drv.so
FD400000   16384   16384       -       -rw-s- 

The input to pixman_blt() seems to be wrong:

   src_bits is 0xfd2d3000
   src_y is 504
   stride is 600 (given in number of uint32_t)

so the first source pixel is at 

    >>> hex (0xfd2d3000 + 504 * 4 * 600)
    '0xfd3fa500L'

which is outside the mapping. The blt routine works left-to-right, so it starts with the last pixel of the first line:

    >>> hex (0xfd2d3000 + 504 * 4 * 600 + 574 * 4)
    '0xfd3fadf8L'

which is the faulting address. 

Nothing jumps out at me from looking at the fbCopyNtoN() source though, so I'd guess the bug is somewhere in the shared memory extension.

This change to shm.c  seems to fix the problem. 

--- /export/home/XW_NV_MWS/open-src/xserver/xorg/build_32/xorg-server-1.6.3/Xext/shm.c	Mon Aug 17 16:29:46 2009
+++ shm.c	Tue Aug 18 14:35:39 2009
@@ -499,8 +499,8 @@
 	      char *data)
 {
     PixmapPtr pPixmap;
-  
-    pPixmap = GetScratchPixmapHeader(dst->pScreen, w, h, depth,
+
+    pPixmap = GetScratchPixmapHeader(dst->pScreen, sw, sh, depth,
 				     BitsPerPixel(depth),
 				     PixmapBytePad(w, depth),
 				     data);

If the fix is correct, the PixmapBytePad() call in there probably also needs to be changed to use sw instead of w - does that still work?

It would be interesting to see the parameters passed to doShmPutImage() when the problem occurs.

#6  0x080d3850 in KdBacktrace (signum=11) at ../../../../hw/kdrive/src/kdrive.c:1313
#7  <signal handler called>
#8  pixman_blt_sse2 (src_bits=<value optimized out>, dst_bits=<value optimized out>, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-sse2.c:5330
#9  0x0409d740 in sse2_blt (imp=0x48fc3d0, src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-sse2.c:5729
#10 0x04065223 in _pixman_implementation_blt (imp=0x48fc3d0, src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-implementation.c:250
#11 0x0408a4b8 in pixman_blt (src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman.c:236
#12 0x080e1010 in fbCopyNtoN (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, pbox=0xbeaa2368, nbox=0, dx=573, dy=573, reverse=0, upsidedown=0, bitplane=0, closure=0x0) at ../../fb/fbcopy.c:64
#13 0x080e1cb0 in fbCopyRegion (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, pDstRegion=0xbeaa2368, dx=573, dy=573, copyProc=0x80e0de0 <fbCopyNtoN>, bitPlane=0, closure=0x0) at ../../fb/fbcopy.c:396
#14 0x080e2159 in fbDoCopy (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, xIn=576, yIn=576, widthSrc=10, heightSrc=10, xOut=3, yOut=3, copyProc=0x80e0de0 <fbCopyNtoN>, bitPlane=0, closure=0x0) at ../../fb/fbcopy.c:596
#15 0x080e22bf in fbCopyArea (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, xIn=576, yIn=576, widthSrc=10, heightSrc=10, xOut=0, yOut=0) at ../../fb/fbcopy.c:634
#16 0x081c969d in damageCopyArea (pSrc=0x51cec30, pDst=0x50dd3a0, pGC=0x51c1ed0, srcx=576, srcy=576, width=10, height=10, dstx=0, dsty=0) at ../../../miext/damage/damage.c:949
#17 0x08134171 in doShmPutImage (dst=0x50dd3a0, pGC=0x51c1ed0, depth=24, format=1, w=600, h=600, sx=576, sy=576, sw=10, sh=10, dx=0, dy=0, data=0x541a000 "") at ../../Xext/shm.c:509
#18 0x0813599d in ProcShmPutImage (client=0x50dba18) at ../../Xext/shm.c:865
#19 0x08136960 in ProcShmDispatch (client=0x50dba18) at ../../Xext/shm.c:1119
#20 0x080b3779 in Dispatch () at ../../dix/dispatch.c:456
#21 0x0809ab46 in main (argc=8, argv=0xbeaa2734, envp=0xbeaa2758) at ../../dix/main.c:397


#7  <signal handler called>
No symbol table info available.
#8  pixman_blt_sse2 (src_bits=<value optimized out>, dst_bits=<value optimized out>, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-sse2.c:5330
	w = 40
	s = (uint8_t *) 0x556c100 <Address 0x556c100 out of bounds>
	d = (uint8_t *) 0x4af400c ""
	src_bytes = (uint8_t *) 0x556c100 <Address 0x556c100 out of bounds>
	dst_bytes = (uint8_t *) 0x4af400c ""
	byte_width = 40
#9  0x0409d740 in sse2_blt (imp=0x48fc3d0, src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-sse2.c:5729
No locals.
#10 0x04065223 in _pixman_implementation_blt (imp=0x48fc3d0, src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman-implementation.c:250
No locals.
#11 0x0408a4b8 in pixman_blt (src_bits=0x541a000, dst_bits=0x4af1000, src_stride=600, dst_stride=1024, src_bpp=32, dst_bpp=32, src_x=576, src_y=576, dst_x=3, dst_y=3, width=10, height=10) at pixman.c:236
No locals.
#12 0x080e1010 in fbCopyNtoN (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, pbox=0xbeaa2368, nbox=0, dx=573, dy=573, reverse=0, upsidedown=0, bitplane=0, closure=0x0) at ../../fb/fbcopy.c:64
	alu = 3 '\003'
	pm = 4294967295
	src = (FbBits *) 0x541a000
	srcStride = 600
	srcBpp = 32
	srcXoff = 0
	srcYoff = 0
	dst = (FbBits *) 0x4af1000
	dstStride = 1024
	dstBpp = 32
	dstXoff = 0
	dstYoff = 0
#13 0x080e1cb0 in fbCopyRegion (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, pDstRegion=0xbeaa2368, dx=573, dy=573, copyProc=0x80e0de0 <fbCopyNtoN>, bitPlane=0, closure=0x0) at ../../fb/fbcopy.c:396
	careful = 0
	reverse = 0
	upsidedown = 0
	pbox = (BoxPtr) 0xbeaa2368
	nbox = 1
	pboxNew1 = (BoxPtr) 0x0
	pboxNew2 = (BoxPtr) 0x0
	pboxBase = (BoxPtr) 0x471c930
	pboxNext = (BoxPtr) 0x0
	pboxTmp = (BoxPtr) 0x49bc4c8
#14 0x080e2159 in fbDoCopy (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, xIn=576, yIn=576, widthSrc=10, heightSrc=10, xOut=3, yOut=3, copyProc=0x80e0de0 <fbCopyNtoN>, bitPlane=0, closure=0x0) at ../../fb/fbcopy.c:596
	prgnSrcClip = (RegionPtr) 0x0
	freeSrcClip = 0
	prgnExposed = (RegionPtr) 0x0
	rgnDst = {extents = {x1 = 3, y1 = 3, x2 = 13, y2 = 13}, data = 0x0}
	dx = 573
	dy = 573
	numRects = 1
	box_x1 = 3
	box_y1 = 3
	box_x2 = 13
	box_y2 = 13
	fastSrc = 1
	fastDst = 1
	fastExpose = 1
#15 0x080e22bf in fbCopyArea (pSrcDrawable=0x51cec30, pDstDrawable=0x50dd3a0, pGC=0x51c1ed0, xIn=576, yIn=576, widthSrc=10, heightSrc=10, xOut=0, yOut=0) at ../../fb/fbcopy.c:634
	copy = (fbCopyProc) 0x80e0de0 <fbCopyNtoN>
#16 0x081c969d in damageCopyArea (pSrc=0x51cec30, pDst=0x50dd3a0, pGC=0x51c1ed0, srcx=576, srcy=576, width=10, height=10, dstx=0, dsty=0) at ../../../miext/damage/damage.c:949
	ret = (RegionPtr) 0x8291ff4
	pGCPriv = (DamageGCPrivPtr) 0x51c2008
	oldFuncs = (GCFuncs *) 0x8299360
#17 0x08134171 in doShmPutImage (dst=0x50dd3a0, pGC=0x51c1ed0, depth=24, format=1, w=600, h=600, sx=576, sy=576, sw=10, sh=10, dx=0, dy=0, data=0x541a000 "") at ../../Xext/shm.c:509
	pPixmap = (PixmapPtr) 0x51cec30
#18 0x0813599d in ProcShmPutImage (client=0x50dba18) at ../../Xext/shm.c:865
	pGC = (GCPtr) 0x51c1ed0
	pDraw = (DrawablePtr) 0x50dd3a0
	length = 1824
	shmdesc = (ShmDescPtr) 0x51cea60
	stuff = (xShmPutImageReq *) 0x50dbca0
#19 0x08136960 in ProcShmDispatch (client=0x50dba18) at ../../Xext/shm.c:1119
	stuff = (xReq *) 0x50dbca0
#20 0x080b3779 in Dispatch () at ../../dix/dispatch.c:456
	clientReady = (int *) 0x50af208
	result = 0
	client = (ClientPtr) 0x50dba18
	nready = 0
	icheck = (HWEventQueuePtr *) 0x82c2fcc
	start_tick = 5380
#21 0x0809ab46 in main (argc=8, argv=0xbeaa2734, envp=0xbeaa2758) at ../../dix/main.c:397
	i = 1
	alwaysCheckForInput = {0, 1}


gdb> f 18
#18 0x0813599d in ProcShmPutImage (client=0x50dba18) at ../../Xext/shm.c:865
gdb> p *stuff
$24 = {reqType = 130 '\202', shmReqType = 3 '\003', length = 10, drawable = 4194305, gc = 4194308, totalWidth = 600, totalHeight = 600, srcX = 576, srcY = 576, srcWidth = 10, srcHeight = 10, dstX = 0, dstY = 0, depth = 24 '\030', format = 1 '\001', sendEvent = 0 '\0', bpad = 0 '\0', shmseg = 4194312, offset = 0}
gdb> p *shmdesc
$25 = {next = 0x0, shmid = 427229334, refcnt = 1, addr = 0x541a000 "", writable = 0, size = 1094400}
gdb> p (double)shmdesc->size / 600 / 600
$26 = 3.04
gdb> p (double)shmdesc->size / 2400
$27 = 456
gdb> p (double)shmdesc->size / 1800
$28 = 608

gdb> f 17
#17 0x08134171 in doShmPutImage (dst=0x50dd3a0, pGC=0x51c1ed0, depth=24, format=1, w=600, h=600, sx=576, sy=576, sw=10, sh=10, dx=0, dy=0, data=0x541a000 "") at ../../Xext/shm.c:509
gdb> p *pPixmap
$29 = {drawable = {type = 1 '\001', class = 0 '\0', depth = 24 '\030', bitsPerPixel = 32 ' ', id = 0, x = 0, y = 0, width = 600, height = 600, pScreen = 0x4717ce8, serialNumber = 20}, devPrivates = 0x0, refcnt = 1, devKind = 2400, devPrivate = {ptr = 0x541a000, val = 88186880, uval = 88186880, fptr = 0x541a000}, screen_x = 0, screen_y = 0, usage_hint = 0}

As you can see, the size of the shm segment is about 600x600x24 bits, so we have 24-bit pixmap data in there. The bitsPerPixel of our scratch pixmap is 32, though. I assume this is the reason for the invalid memory reads.
With 16 bit color depth, the xserver does not crash but the x11perf output is distorted.

This bug was introduced by:

commit ee7c684f21d220d5e046bab31ae617a7d64d60f6
Author: Adam Jackson <ajax@redhat.com>
Date:   Wed Aug 27 16:05:47 2008 -0400

    Reimplement ShmPutImage.

Reverting the changes to Xext/shm* (I didn't bother to resolve the merge conflicts in exa/*) lets -shmputxy10 finish in Xephyr without valgrind moaning and without graphical glitches.

Created attachment 29650 [details] [review]
My test patch that reverts the faulty changes

Just in case someone wants to test this. No idea whether EXA still works, though. :)

Removing from 7.5 tracker, the 1.7 tracker is enough.

Created attachment 29667 [details] [review]
Possible fix

Does this patch work? It just uses the old code for the non-ZPixmap case. There might be a better solution though.

(In reply to comment #8)
> Does this patch work? It just uses the old code for the non-ZPixmap case. There
> might be a better solution though. 

server doesn't crash anymore with this patch applied. I think this is good enough as a workaround for now - please push if you think the patch is ready.

(In reply to comment #9)
> I think this is good enough as a workaround for now - please push if you think
> the patch is ready.

Thanks, but I'd like to know Adam's opinion first.

Ajax - any comments on the patch?

(In reply to comment #8)
> Created an attachment (id=29667) [details]
> Possible fix
> 
> Does this patch work? It just uses the old code for the non-ZPixmap case. There
> might be a better solution though.

Looks fine to me, ACK.

Michel, please push this to master and I'll cherry-pick it for 1.7.

Fix pushed to master as 11817a881cb93a89788105d1e575a468f2a8d27c .