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 <[email protected]> > > Signed-off-by: Erkki Seppälä <[email protected]> > Reviewed-by: Oliver McFadden <[email protected]> Reported-by: Phil Carmody <[email protected]> Tested-by: Phil Carmody <[email protected]> > > --- > > 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 > > > > _______________________________________________ > > [email protected]: X.Org development > > Archives: http://lists.x.org/archives/xorg-devel > > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
