[PATCH] fix doPolyText use-after-free issue

[Alan Hourihane]

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
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dixfonts.c.patch
Type: text/x-patch
Size: 2058 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20111004/83a7ec95/attachment.bin>