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

Jamey Sharp jamey at minilop.net
Fri May 14 11:19:36 PDT 2010


On Fri, May 14, 2010 at 11:00 AM, Jeremy Huddleston <jeremyhu at apple.com> wrote:
> 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.

Yeah, I'm noticing that.

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

With an always-fail hack in CreateGC, I see what I think is basically
the same bug, except that the crash is in miSpriteRealizeCursor.

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

I have a patch for this: replace the malloc call with calloc so
cleanup can run safely.

In both cases, the first question is why CreateGC is still failing for
you, so let's look at that:

> 420         gcvals[0] = IncludeInferiors;
> 421         gcvals[1] = FALSE;
> 422         pGC = CreateGC((DrawablePtr)pWin,
> 423                        GCSubwindowMode|GCGraphicsExposures, gcvals, &status,
> 424                        (XID)0, serverClient);
>
> 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
>  }, ...}

vals[1].val correctly got set to FALSE, but vals[0].val did not get
set to IncludeInferiors (1).

I'd expect to see this result with yesterday's master. Are you sure
you're testing with my patch? :-)

Jamey


More information about the xorg-devel mailing list