[PATCH] [xserver] record: avoid crash when calling RecordFlushReplyBuffer recursively

[Oliver McFadden]

On Mon, 2011-02-14 at 19:59 +0200, Oliver McFadden wrote:
> I don't think this has been merged into master yet (and we should apply
> it to the 1.9.xxx branches too), so I've added my Reviewed-by below.
> Good catch!

Sorry for the bump again. I was asked to add the Reported-by and
Tested-by tags on behalf of Phil because he's not an X developer and
didn't want to subscribe here.

> On Thu, 2011-02-10 at 15:35 +0200, ext Erkki Seppälä wrote:
> > RecordFlushReplyBuffer can call itself recursively through
> > WriteClient->CallCallbacks->_CallCallbacks->RecordFlushAllContexts
> > when the recording client's buffer cannot be completely emptied in one
> > WriteClient. When a such a recursion occurs, it will not be broken out
> > of which results in segmentation fault when the stack is exhausted.
> > 
> > This patch adds a counter (a flag, really) that guards against this
> > situation, to break out of the recursion.
> > 
> > One alternative to this change would be to change _CallCallbacks to
> > check the corresponding counter before the callback loop, but that
> > might affect existing behavior, which may be relied upon.
> > 
> > Reviewed-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
> > Signed-off-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> Reviewed-by: Oliver McFadden <oliver.mcfadden at nokia.com>
Reported-by: Phil Carmody <ext-phil.2.carmody at nokia.com>
Tested-by: Phil Carmody <ext-phil.2.carmody at nokia.com>
> > ---
> >  record/record.c |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/record/record.c b/record/record.c
> > index 6a93d7a..bea3046 100644
> > --- a/record/record.c
> > +++ b/record/record.c
> > @@ -77,6 +77,7 @@ typedef struct {
> >      char	bufCategory;	   /* category of protocol in replyBuffer */
> >      int		numBufBytes;	   /* number of bytes in replyBuffer */
> >      char	replyBuffer[REPLY_BUF_SIZE]; /* buffered recorded protocol */
> > +    int		inFlush;           /*  are we inside RecordFlushReplyBuffer */
> >  } RecordContextRec, *RecordContextPtr;
> >  
> >  /*  RecordMinorOpRec - to hold minor opcode selections for extension requests
> > @@ -245,8 +246,9 @@ RecordFlushReplyBuffer(
> >      int len2
> >  )
> >  {
> > -    if (!pContext->pRecordingClient || pContext->pRecordingClient->clientGone) 
> > +    if (!pContext->pRecordingClient || pContext->pRecordingClient->clientGone || pContext->inFlush) 
> >  	return;
> > +    ++pContext->inFlush;
> >      if (pContext->numBufBytes)
> >  	WriteToClient(pContext->pRecordingClient, pContext->numBufBytes,
> >  		      (char *)pContext->replyBuffer);
> > @@ -255,6 +257,7 @@ RecordFlushReplyBuffer(
> >  	WriteToClient(pContext->pRecordingClient, len1, (char *)data1);
> >      if (len2)
> >  	WriteToClient(pContext->pRecordingClient, len2, (char *)data2);
> > +    --pContext->inFlush;
> >  } /* RecordFlushReplyBuffer */
> >  
> > 
> > @@ -1938,6 +1941,7 @@ ProcRecordCreateContext(ClientPtr client)
> >      pContext->numBufBytes = 0;
> >      pContext->pBufClient = NULL;
> >      pContext->continuedReply = 0;
> > +    pContext->inFlush = 0;
> >  
> >      err = RecordRegisterClients(pContext, client,
> >  				(xRecordRegisterClientsReq *)stuff);
> > -- 
> > 1.7.0.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
>