[PATCH] fix doPolyText use-after-free issue

[Jeremy Huddleston]

Thanks,  I've pulled this into my tree, and it will be in my next pull request to master.

On Oct 4, 2011, at 12:48 AM, Alan Hourihane wrote:

> Attached.
> 
> Thanks Jeremy. What's happening to the 1.10.x branch now ? Can this be
> nominated for that too ?
> 
> Alan.
> 
> On 09/28/11 07:46, Jeremy Huddleston wrote:
>> I missed this point at first.  The context is that in 'bail', c is accessed and expected to be the old value.
>> 
>> Candidate for 1.11-branch
>> 
>> Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
>> 
>> I think something is wrong with your mailer (or maybe mine).  I was unable to git-am this patch.  Can you please resend it as an attachment, and I'll git-am it into my tree.
>> 
>> --Jeremy
>> 
>> On Sep 27, 2011, at 6:51 AM, Alan Hourihane wrote:
>> 
>>> dixfonts: Don't overwrite local c variable until new_closure is safely
>>> initialized.
>>> 
>>> Signed-off-by: Alan Hourihane <alanh at vmware.com>
>>> 
>>> diff --git a/dix/dixfonts.c b/dix/dixfonts.c
>>> index fbac124..d2bcb84 100644
>>> --- a/dix/dixfonts.c
>>> +++ b/dix/dixfonts.c
>>> @@ -1302,31 +1302,30 @@ doPolyText(ClientPtr client, PTclosurePtr c)
>>>            goto bail;
>>>            }
>>>            *new_closure = *c;
>>> -            c = new_closure;
>>> 
>>> -            len = c->endReq - c->pElt;
>>> -            c->data = malloc(len);
>>> -            if (!c->data)
>>> +            len = new_closure->endReq - new_closure->pElt;
>>> +            new_closure->data = malloc(len);
>>> +            if (!new_closure->data)
>>>            {
>>> -            free(c);
>>> +            free(new_closure);
>>>            err = BadAlloc;
>>>            goto bail;
>>>            }
>>> -            memmove(c->data, c->pElt, len);
>>> -            c->pElt = c->data;
>>> -            c->endReq = c->pElt + len;
>>> +            memmove(new_closure->data, new_closure->pElt, len);
>>> +            new_closure->pElt = new_closure->data;
>>> +            new_closure->endReq = new_closure->pElt + len;
>>> 
>>>            /* Step 2 */
>>> 
>>> -            pGC = GetScratchGC(c->pGC->depth, c->pGC->pScreen);
>>> +            pGC = GetScratchGC(new_closure->pGC->depth,
>>> new_closure->pGC->pScreen);
>>>            if (!pGC)
>>>            {
>>> -            free(c->data);
>>> -            free(c);
>>> +            free(new_closure->data);
>>> +            free(new_closure);
>>>            err = BadAlloc;
>>>            goto bail;
>>>            }
>>> -            if ((err = CopyGC(c->pGC, pGC, GCFunction |
>>> +            if ((err = CopyGC(new_closure->pGC, pGC, GCFunction |
>>>                      GCPlaneMask | GCForeground |
>>>                      GCBackground | GCFillStyle |
>>>                      GCTile | GCStipple |
>>> @@ -1337,15 +1336,16 @@ doPolyText(ClientPtr client, PTclosurePtr c)
>>>                      Success)
>>>            {
>>>            FreeScratchGC(pGC);
>>> -            free(c->data);
>>> -            free(c);
>>> +            free(new_closure->data);
>>> +            free(new_closure);
>>>            err = BadAlloc;
>>>            goto bail;
>>>            }
>>> +            c = new_closure;
>>>            origGC = c->pGC;
>>>            c->pGC = pGC;
>>>            ValidateGC(c->pDraw, c->pGC);
>>> -            
>>> +
>>>            ClientSleep(client, (ClientSleepProcPtr)doPolyText, c);
>>> 
>>>            /* Set up to perform steps 3 and 4 */
>>> 
>>> _______________________________________________
>>> xorg-devel at lists.x.org: X.Org development
>>> Archives: http://lists.x.org/archives/xorg-devel
>>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>> 
> 
> <dixfonts.c.patch>

---
Jeremy Huddleston

Rebuild Sudan
 - Board of Directors
 - http://www.rebuildsudan.org

Berkeley Foundation for Opportunities in Information Technology
 - Advisory Board
 - http://www.bfoit.org