[PATCH] record: Prevent a crash on recording client disconnect.

[Rami Ylimäki]

Execute the following steps to reproduce the issue.

  1. Run at least two recording clients simultaneously.
     $ cnee --record --request-range 1-127 &
     $ cnee --record --request-range 1-127 &
  2. Kill the recording clients.
     $ killall cnee
  3. Give X server something to do so that the clients are closed.
     $ xinput list
     $ xinput list

As a result RecordUninstallHooks accesses NullClient, because
RecordAClientStateChange doesn't clean the recording clients up
properly.

Fix RecordUninstallHooks to fail locally on an assertion instead of
much later in privates code, if NullClient is still accessed because
of some other bug. Fix RecordAClientStateChange to iterate through all
contexts so that modifications of the iterated array during iteration
don't cause contexts to be skipped.

Signed-off-by: Rami Ylimäki <ext-rami.ylimaki at nokia.com>
---
 record/record.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/record/record.c b/record/record.c
index 06006f7..6a93d7a 100644
--- a/record/record.c
+++ b/record/record.c
@@ -997,10 +997,11 @@ RecordUninstallHooks(RecordClientsAndProtocolPtr pRCAP, XID oneclient)
 		ClientPtr pClient = clients[CLIENT_ID(client)];
 		int c;
 		Bool otherRCAPwantsProcVector = FALSE;
-		RecordClientPrivatePtr pClientPriv =
-						RecordClientPrivate(pClient);
+		RecordClientPrivatePtr pClientPriv = NULL;
 
-		assert (pClient && RecordClientPrivate(pClient));
+		assert (pClient);
+		pClientPriv = RecordClientPrivate(pClient);
+		assert (pClientPriv);
 		memcpy(pClientPriv->recordVector, pClientPriv->originalVector,
 		       sizeof (pClientPriv->recordVector));
 
@@ -2813,6 +2814,8 @@ RecordAClientStateChange(CallbackListPtr *pcbl, pointer nulldata, pointer callda
     NewClientInfoRec *pci = (NewClientInfoRec *)calldata;
     int i;
     ClientPtr pClient = pci->client;
+    RecordContextPtr *ppAllContextsCopy = NULL;
+    int numContextsCopy = 0;
 
     switch (pClient->clientState)
     {
@@ -2834,10 +2837,17 @@ RecordAClientStateChange(CallbackListPtr *pcbl, pointer nulldata, pointer callda
 
     case ClientStateGone:
     case ClientStateRetained: /* client disconnected */
-	for (i = 0; i < numContexts; i++)
+
+        /* RecordDisableContext modifies contents of ppAllContexts. */
+	numContextsCopy = numContexts;
+	ppAllContextsCopy = malloc(numContextsCopy * sizeof(RecordContextPtr));
+	assert(ppAllContextsCopy);
+	memcpy(ppAllContextsCopy, ppAllContexts, numContextsCopy * sizeof(RecordContextPtr));
+
+	for (i = 0; i < numContextsCopy; i++)
 	{
 	    RecordClientsAndProtocolPtr pRCAP;
-	    RecordContextPtr pContext = ppAllContexts[i];
+	    RecordContextPtr pContext = ppAllContextsCopy[i];
 	    int pos;
 
 	    if (pContext->pRecordingClient == pClient)
@@ -2851,6 +2861,8 @@ RecordAClientStateChange(CallbackListPtr *pcbl, pointer nulldata, pointer callda
 		RecordDeleteClientFromRCAP(pRCAP, pos);
 	    }
 	}
+
+	free(ppAllContextsCopy);
     break;
 
     default:
-- 
1.6.3.3