[PATCH] glx: Pass GLX drawable ID to DRI2CreateDrawable().

[Kristian Høgsberg]

On Fri, Jun 29, 2012 at 12:30 PM, Michel Dänzer <michel at daenzer.net> wrote:
> [ Did you intentionally not Cc the xorg-devel list? ]

No that was an accident... not sure why I have "reply all" as default...

> On Fre, 2012-06-29 at 12:20 -0400, Kristian Høgsberg wrote:
>> On Thu, Jun 28, 2012 at 7:39 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> > From: Michel Dänzer <michel.daenzer at amd.com>
>> >
>> > Otherwise the DRI2Drawable may retain references to the destroyed
>> > __GLXDRIdrawable, leading to use after free in __glXDRIinvalidateBuffers().
>>
>> That looks wrong to me.  DRI2 isn't concerned with GLX drawables, only
>> X drawables.  If you're destroying the GLX drawable and want to not
>> get invalidate callbacks, you need to destroy the DRI2DrawableRef that
>> DRI2CreateDrawable creates.
>
> Which is what this patch does? :) (By means of
> glxcmds.c:DoDestroyDrawable -> FreeResource -> DRI2DrawableGone, where
> the ID matches ref->id, so it calls
> FreeResourceByType(ref->dri2_id, ...) as well)
>
> Can you explain why the non-GLX drawable ID needs to be passed to
> DRI2CreateDrawable?

The DRI2Drawable is created for the X drawable, not the GLX drawable.
When the X drawable goes away the DRI2 drawable needs to go away.  It
works the way it does, since a pixmap can have multiple XIDs and for
each XID, mutliple clients could call DRI2CreateClient.  We need to
keep the DRI2 drawable alive for each reference for each XID.  DRI2
automatically reclaims the DRI2Drawable when the underlying X drawable
is destroyed, but that will break if you pass in the GLX drawable XID.
 What's missing now is a way to destroy the DRI2DrawableRef when you
destroy the GLX drawable.  I'm suggesting we add that by returning the
dri2_id so glx can FreeResource that.

Maybe it could work the way you describe, but that's not how it was
designed to work and I can't immediately say that it won't break some
subtle way.

Kristian

>
>
> --
> Earthling Michel Dänzer           |                   http://www.amd.com
> Libre software enthusiast         |          Debian, X and DRI developer