[PATCH v2 08/10] dri2: Send events only to known clients

[Ville Syrjälä]

On Tue, Feb 15, 2011 at 09:18:36AM +0200, Pauli Nieminen wrote:
> On 14/02/11 21:06 +0200, Ville Syrjälä wrote:
> > On Tue, Feb 08, 2011 at 11:42:54PM +0200, ext Pauli wrote:
> > > From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > > 
> > > If client disconnects and new client gets same id DRI2 events may end to
> > > wrong client. DRI2 reference list can be checked to see if the client
> > > still owns the DRI2Drawable.
> > > 
> > > Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > > ---
> > >  hw/xfree86/dri2/dri2.c |   73 +++++++++++++++++++++++++++++++++++------------
> > >  1 files changed, 54 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> > > index 9133bb3..c77ac9b 100644
> > > --- a/hw/xfree86/dri2/dri2.c
> > > +++ b/hw/xfree86/dri2/dri2.c
> > > @@ -111,12 +111,16 @@ typedef struct _DRI2Screen {
> > >      ConfigNotifyProcPtr		 ConfigNotify;
> > >  } DRI2ScreenRec;
> > >  
> > > +typedef struct _DRI2ClientEventRec {
> > > +    ClientPtr		client;
> > > +    struct list		link;
> > > +} DRI2ClientEventRec, *DRI2ClientEventPtr;
> > > +
> > >  typedef struct _DRI2SwapCompleteDataRec {
> > >      DRI2BufferPtr	src;
> > >      DRI2BufferPtr	dest;
> > > -    ClientPtr		client;
> > >      void *		data;
> > > -    struct list		link;
> > > +    DRI2ClientEventRec	event;
> > >  } DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr;
> > >  
> > >  static struct list *
> > > @@ -266,7 +270,8 @@ DRI2LookupClientDrawableRef(DRI2DrawablePtr pPriv, ClientPtr client, XID id)
> > >      DRI2DrawableRefPtr ref;
> > >  
> > >      list_for_each_entry(ref, &pPriv->reference_list, link) {
> > > -	if (CLIENT_ID(ref->dri2_id) == client->index && ref->id == id)
> > > +	if (CLIENT_ID(ref->dri2_id) == client->index &&
> > > +		(id == 0 || ref->id == id))
> > >  	    return ref;
> > >      }
> > >      return NULL;
> > > @@ -753,28 +758,41 @@ DRI2CanExchange(DrawablePtr pDraw)
> > >      return FALSE;
> > >  }
> > >  
> > > +static void
> > > +cleanup_client_event(DRI2ClientEventPtr event)
> > > +{
> > > +    if (event->client)
> > > +	list_del(&event->link);
> > > +}
> > > +
> > >  void
> > >  DRI2WaitMSCComplete2(DRI2DrawablePtr pPriv, int frame,
> > >  		     unsigned int tv_sec, unsigned int tv_usec,
> > >  		     void *data)
> > >  {
> > > -    ClientPtr client = data;
> > > +    ClientPtr          blockedClient = pPriv->blockedClient;
> > > +    DRI2ClientEventPtr pEvent = data;
> > > +    ClientPtr          client = pEvent->client;
> > >  
> > > +    pPriv->blockedClient = NULL;
> > > +    pPriv->blockedOnMsc = FALSE;
> > >      pPriv->refcnt--;
> > >  
> > > -    if (pPriv->refcnt <= 0) {
> > > -	DRI2DrawableGone(pPriv, 0);
> > > -	return;
> > > -    }
> > > +    if (client == NULL)
> > > +	goto out;
> > >  
> > >      ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec,
> > >  			 frame, pPriv->swap_count);
> > >  
> > > -    if (pPriv->blockedClient)
> > > -	AttendClient(pPriv->blockedClient);
> > > +    if (blockedClient)
> > > +	AttendClient(blockedClient);
> > >  
> > > -    pPriv->blockedClient = NULL;
> > > -    pPriv->blockedOnMsc = FALSE;
> > > +out:
> > > +    cleanup_client_event(pEvent);
> > > +    free(pEvent);
> > > +
> > > +    if (pPriv->refcnt <= 0)
> > > +	DRI2DrawableGone(pPriv, 0);
> > >  }
> > >  
> > >  static void
> > > @@ -816,7 +834,7 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
> > >  static void
> > >  free_swap_complete_data (DRI2DrawablePtr pPriv, DRI2SwapCompleteDataPtr pSwapData)
> > >  {
> > > -    list_del(&pSwapData->link);
> > > +    cleanup_client_event(&pSwapData->event);
> > >      buffer_unref(pPriv, pSwapData->src);
> > >      buffer_unref(pPriv, pSwapData->dest);
> > >      free(pSwapData);
> > > @@ -828,9 +846,10 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
> > >  		   DRI2SwapEventPtr swap_complete, void *swap_data)
> > >  {
> > >      DRI2SwapCompleteDataPtr pSwapData = swap_data;
> > > -    ClientPtr               client = pSwapData->client;
> > > +    ClientPtr               client = pSwapData->event.client;
> > >      DrawablePtr             pDraw = pPriv->drawable;
> > >      CARD64                  ust = 0;
> > > +    DRI2DrawableRefPtr      ref;
> > >  
> > >      pPriv->swapsPending--;
> > >      pPriv->swap_count++;
> > > @@ -841,6 +860,11 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
> > >      if (client == NULL)
> > >  	goto out;
> > >  
> > > +    ref = DRI2LookupClientDrawableRef(pPriv, client, 0);
> > > +
> > > +    if (ref == NULL)
> > > +	goto out;
> > 
> > This bit escapes me. pSwapData->event.client is not NULL, so we know
> > the original client is still around. Are we now assuming that
> > DRI2SwapComplete2 was somehow called with an incorrect pSwapData
> > since we have to go looking for the client's ID in the ref list?
> > 
> 
> We still have to check that client haven't called DRIDestroyDrawable. In that
> case client shouldn't receive "random" DRI2 events.

Ah, OK. So there were two changes in the patch. Could be split up I
suppose.

-- 
Ville Syrjälä