[PATCH] glx: Fix use after free in DrawableGone

[Kristian Høgsberg]

2010/9/23 Jon TURNEY <jon.turney at dronecode.org.uk>:
> On 23/09/2010 13:50, Kristian Høgsberg wrote:
>>
>> Signed-off-by: Kristian Høgsberg<krh at bitplanet.net>
>> ---
>>
>>> ==2989== Invalid write of size 4
>>> ==2989==    at 0x48CE6E5: DrawableGone (glxext.c:169)
>>> ==2989==    by 0x809F401: FreeResource (resource.c:601)
>>> ==2989==    by 0x80845CE: ProcDestroyWindow (dispatch.c:733)
>>> ==2989==    by 0x8087D76: Dispatch (dispatch.c:432)
>>> ==2989==    by 0x8066439: main (main.c:291)
>>> ==2989==  Address 0x55a9c1c is 76 bytes inside a block of size 88 free'd
>>> ==2989==    at 0x4023B6A: free (vg_replace_malloc.c:366)
>>> ==2989==    by 0x48D9DD8: __glXDRIcontextDestroy (glxdri2.c:250)
>>> ==2989==    by 0x48CE1A0: __glXFreeContext (glxext.c:222)
>>> ==2989==    by 0x48CE786: DrawableGone (glxext.c:165)
>>> ==2989==    by 0x809F401: FreeResource (resource.c:601)
>>> ==2989==    by 0x80845CE: ProcDestroyWindow (dispatch.c:733)
>>> ==2989==    by 0x8087D76: Dispatch (dispatch.c:432)
>>> ==2989==    by 0x8066439: main (main.c:291)
>>>
>>> Kristian, I know how much you enjoy fixing these... You probably already
>>> have a patch. ;-)
>>
>> I think we're looking at this use after free problem.  We may free the
>> context in the big if-statement above and the go and touch its drawPriv
>> and readPriv fields afterwards.
>
> I have some difficulty in understanding how it is correct to free the
> context here at all.
>
> See https://bugs.freedesktop.org/show_bug.cgi?id=30089

We only free it when both c->idExists and c->isCurrent are false.
Specifically, when c->idExists is false, it means that the client has
already destroyed the context and we're only holding on to it because
it's still current.  When the window it was current for is destroy the
context is no longer current and we're free to destroy it.  If think
the problem in 30089 is that we don't clear the tag client side when
we destroy the context.

Kristian