[PATCH] Bugfix for "Pre-validate ChangeGC XIDs": off-by-one in loop index.

[Jeremy Huddleston]

Well, the issue is still persistent for my case.  The problem is difficult to discover because multiple points along the route have poor error handling.

1) ActivateDevice needs to handle the case where miPointerDeviceInitialize returns FALSE or we will crash in miPointerConstrainCursor.

271	    if (!((*pScreenPriv->spriteFuncs->DeviceCursorInitialize)(pDev, pScreen)))
(gdb) 
273	        free(pPointer);
(gdb) 
274	        return FALSE;
(gdb) 
279	}

...

Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000020
0x0000000100198694 in miPointerConstrainCursor (pDev=0x103046ad0, pScreen=0x103208b00, pBox=0x110c54f10) at mipointer.c:208
208	    pPointer->limits = *pBox;
(gdb) print pPointer
$1 = (miPointerPtr) 0x0

2) miDCDeviceInitialize's failure and call to miDCDeviceCleanup can crash because miDCDeviceCleanup does not validate input:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000053
0x000000010015b7fb in FreeGC (value=0x3, gid=0) at gc.c:786
786	    CloseFont(pGC->font, (Font)0);
(gdb) bt
#0  0x000000010015b7fb in FreeGC (value=0x3, gid=0) at gc.c:786
#1  0x00000001001922de in miDCDeviceCleanup (pDev=0x111299580, pScreen=0x10d605fb0) at midispcur.c:859
#2  0x00000001001921cf in miDCDeviceInitialize (pDev=0x111299580, pScreen=0x10d605fb0) at midispcur.c:837
#3  0x00000001001a2f60 in miSpriteDeviceCursorInitialize (pDev=0x111299580, pScreen=0x10d605fb0) at misprite.c:943
#4  0x0000000100198834 in miPointerDeviceInitialize (pDev=0x111299580, pScreen=0x10d605fb0) at mipointer.c:271
#5  0x00000001001389a6 in ActivateDevice (dev=0x111299580, sendevent=1 '\001') at devices.c:470
#6  0x0000000100138da7 in InitCoreDevices () at devices.c:603
#7  0x0000000100130039 in dix_main (argc=4, argv=0x7fff5fbfdb20, envp=0x7fff5fbfd9c0) at main.c:254
#8  0x0000000100018d6f in server_thread (arg=0x102b07860) at quartzStartup.c:63
#9  0x00007fff879fb456 in _pthread_start ()
#10 0x00007fff879fb309 in thread_start ()

or

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000495a80000
0x0000000100004580 in CloseFont (value=0x495a80000, fid=0) at dixfonts.c:486
486	    if (--pfont->refcnt == 0) {
(gdb) bt
#0  0x0000000100004580 in CloseFont (value=0x495a80000, fid=0) at dixfonts.c:486
#1  0x000000010015b809 in FreeGC (value=0x102861ce6, gid=0) at gc.c:786
#2  0x00000001001922de in miDCDeviceCleanup (pDev=0x1106707d0, pScreen=0x101506f10) at midispcur.c:859
#3  0x00000001001921cf in miDCDeviceInitialize (pDev=0x1106707d0, pScreen=0x101506f10) at midispcur.c:837
#4  0x00000001001a2f60 in miSpriteDeviceCursorInitialize (pDev=0x1106707d0, pScreen=0x101506f10) at misprite.c:943
#5  0x0000000100198834 in miPointerDeviceInitialize (pDev=0x1106707d0, pScreen=0x101506f10) at mipointer.c:271
#6  0x00000001001389a6 in ActivateDevice (dev=0x1106707d0, sendevent=1 '\001') at devices.c:470
#7  0x0000000100138da7 in InitCoreDevices () at devices.c:603
#8  0x0000000100130039 in dix_main (argc=4, argv=0x7fff5fbfdb20, envp=0x7fff5fbfd9c0) at main.c:254
#9  0x0000000100018d6f in server_thread (arg=0x1028704d0) at quartzStartup.c:63
#10 0x00007fff879fb456 in _pthread_start ()
#11 0x00007fff879fb309 in thread_start ()


I traced this to miDCMakeGC:

790	        pBuffer->pSourceGC = miDCMakeGC(pWin);

which is just a call to CreateGC:

(gdb) list
417	    int   status;
418	    XID   gcvals[2];
419	
420	    gcvals[0] = IncludeInferiors;
421	    gcvals[1] = FALSE;
422	    pGC = CreateGC((DrawablePtr)pWin,
423			   GCSubwindowMode|GCGraphicsExposures, gcvals, &status,
424			   (XID)0, serverClient);
425	    return pGC;
426	}


CreateGC fails in the call to dixChangeGC in this block:

557	    if (!(*pGC->pScreen->CreateGC)(pGC))
558		*pStatus = BadAlloc;
559	    else if (mask)
560	        *pStatus = dixChangeGC(client, pGC, mask, pval, NULL);
561	    else
562		*pStatus = Success;

which eventually lands us here:

#0  ChangeGCXIDs (client=0x103f0f9c0, pGC=0x1101311f0, mask=98304, pC32=0x103edac90) at gc.c:438
#1  0x000000010015ab7e in dixChangeGC (client=0x103f0f9c0, pGC=0x1101311f0, mask=98304, pC32=0x103edac90, pUnion=0x0) at gc.c:473
#2  0x000000010015ae7a in CreateGC (pDrawable=0x10074cc00, mask=98304, pval=0x103edac90, pStatus=0x103edacac, gcid=0, client=0x103f0f9c0) at gc.c:560



466	    return ChangeGC(client, pGC, mask, vals);
(gdb) print vals
$3 = {{
    val = 2750440, 
    ptr = 0x10029f7e8
  }, {
    val = 0, 
    ptr = 0x100000000
  }, {
    val = 2750440, 
    ptr = 0x10029f7e8
  }, {
    val = 269685576, 
    ptr = 0x110131348
  }, {
    val = 269685728, 
    ptr = 0x1101313e0
  }, {
    val = 269685568, 
    ptr = 0x110131340
  }, {
    val = 2750444, 
    ptr = 0x10029f7ec
  }, {
    val = 19121776, 
    ptr = 0x10123c670
  }, {
    val = 65907568, 
    ptr = 0x103edab70
  }, {
    val = 1451808, 
    ptr = 0x100162720
  }, {
    val = 19125072, 
    ptr = 0x10123d350
  }, {
    val = 269685728, 
    ptr = 0x1101313e0
  }, {
    val = 2750444, 
    ptr = 0x10029f7ec
  }, {
    val = 19121776, 
    ptr = 0x10123c670
  }, {
    val = 19121120, 
    ptr = 0x10123c3e0
  }, {
    val = 269685576, 
    ptr = 0x110131348
  }, {
    val = 65907648, 
    ptr = 0x103edabc0
  }, {
    val = 136390, 
    ptr = 0x1000214c6
  }, {
    val = 2750320, 
    ptr = 0x10029f770
  }, {
    val = 269685232, 
    ptr = 0x1101311f0
  }, {
    val = 19124704, 
    ptr = 0x10123d1e0
  }, {
    val = 19125072, 
    ptr = 0x10123d350
  }, {
    val = 269685728, 
    ptr = 0x1101313e0
  }}


And here's the failure inside ChangeGC:

ChangeGC (client=0x103f0f9c0, pGC=0x1101311f0, mask=98304, pUnion=0x103edaaf0) at gc.c:140
140	    int 	error = 0;
(gdb) 
144	    assert(pUnion);
(gdb) 
145	    pGC->serialNumber |= GC_CHANGE_SERIAL_BIT;
(gdb) 
147	    maskQ = mask;	/* save these for when we walk the GCque */
(gdb) 
148	    while (mask && !error) 
(gdb) 
150		index2 = (BITS32) lowbit (mask);
(gdb) 
151		mask &= ~index2;
(gdb) 
152		pGC->stateChanges |= index2;
(gdb) 
153		switch (index2)
(gdb) 
303			NEXTVAL(unsigned int, newclipmode);
(gdb) 
304			if (newclipmode <= IncludeInferiors)
(gdb) 
308			    clientErrorValue = newclipmode;
(gdb) 
309			    error = BadValue;
(gdb) 
311			break;
(gdb) 
148	    while (mask && !error) 
(gdb)
407	    if (pGC->fillStyle == FillTiled && pGC->tileIsPixel)
(gdb) 
415	    (*pGC->funcs->ChangeGC)(pGC, maskQ);
(gdb) 
416	    return error;