[PATCH] glx: Fix use after free in DrawableGone

[Jon TURNEY]

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

>
> Kristian
>
>   glx/glxext.c |    6 ++----
>   1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/glx/glxext.c b/glx/glxext.c
> index e203156..0b04c37 100644
> --- a/glx/glxext.c
> +++ b/glx/glxext.c
> @@ -160,15 +160,13 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid)
>   		    }
>   		}
>   	    }
> -
> -	    if (!c->idExists) {
> -		__glXFreeContext(c);
> -	    }
>   	}
>   	if (c->drawPriv == glxPriv)
>   	    c->drawPriv = NULL;
>   	if (c->readPriv == glxPriv)
>   	    c->readPriv = NULL;
> +	if (!c->idExists&&  !c->isCurrent)
> +	    __glXFreeContext(c);
>       }
>
>       glxPriv->destroy(glxPriv);