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

[Oliver McFadden]

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!

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>
> ---
>  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