Any pad bytes in replies are written to the client from a zeroed array. However, record extension tries to incorrectly access the pad bytes from the end of reply data.
Signed-off-by: Rami Ylimäki <[email protected]> Reviewed-by: Erkki Seppälä <[email protected]> --- v1: Fixed the issue when a reply was cached into a recording context buffer. v2: Fixes the issue also for large replies that don't fit into the cache. include/os.h | 3 ++- os/io.c | 1 + record/record.c | 53 ++++++++++++++++++++++++++++++----------------------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/include/os.h b/include/os.h index 5401ea4..fda0181 100644 --- a/include/os.h +++ b/include/os.h @@ -451,9 +451,10 @@ extern _X_EXPORT CallbackListPtr ReplyCallback; typedef struct { ClientPtr client; const void *replyData; - unsigned long dataLenBytes; + unsigned long dataLenBytes; /* actual bytes from replyData + pad bytes */ unsigned long bytesRemaining; Bool startOfReply; + unsigned long padBytes; /* pad bytes from zeroed array */ } ReplyInfoRec; /* stuff for FlushCallback */ diff --git a/os/io.c b/os/io.c index 068f5f0..955bf8b 100644 --- a/os/io.c +++ b/os/io.c @@ -809,6 +809,7 @@ WriteToClient (ClientPtr who, int count, const void *__buf) replyinfo.client = who; replyinfo.replyData = buf; replyinfo.dataLenBytes = count + padBytes; + replyinfo.padBytes = padBytes; if (who->replyBytesRemaining) { /* still sending data of an earlier reply */ who->replyBytesRemaining -= count + padBytes; diff --git a/record/record.c b/record/record.c index 68311ac..db77b64 100644 --- a/record/record.c +++ b/record/record.c @@ -269,8 +269,9 @@ RecordFlushReplyBuffer( * device events and EndOfData, pClient is NULL. * category is the category of the protocol element, as defined * by the RECORD spec. - * data is a pointer to the protocol data, and datalen is its length - * in bytes. + * data is a pointer to the protocol data, and datalen - padlen + * is its length in bytes. + * padlen is the number of pad bytes from a zeroed array. * futurelen is the number of bytes that will be sent in subsequent * calls to this function to complete this protocol element. * In those subsequent calls, futurelen will be -1 to indicate @@ -290,7 +291,7 @@ RecordFlushReplyBuffer( */ static void RecordAProtocolElement(RecordContextPtr pContext, ClientPtr pClient, - int category, pointer data, int datalen, int futurelen) + int category, pointer data, int datalen, int padlen, int futurelen) { CARD32 elemHeaderData[2]; int numElemHeaders = 0; @@ -398,15 +399,20 @@ RecordAProtocolElement(RecordContextPtr pContext, ClientPtr pClient, } if (datalen) { + static char padBuffer[3]; /* as in FlushClient */ memcpy(pContext->replyBuffer + pContext->numBufBytes, - data, datalen); - pContext->numBufBytes += datalen; + data, datalen - padlen); + pContext->numBufBytes += datalen - padlen; + memcpy(pContext->replyBuffer + pContext->numBufBytes, + padBuffer, padlen); + pContext->numBufBytes += padlen; } } else + { RecordFlushReplyBuffer(pContext, (pointer)elemHeaderData, - numElemHeaders, (pointer)data, datalen); - + numElemHeaders, (pointer)data, datalen - padlen); + } } /* RecordAProtocolElement */ @@ -483,19 +489,19 @@ RecordABigRequest(RecordContextPtr pContext, ClientPtr client, xReq *stuff) /* record the request header */ bytesLeft = client->req_len << 2; RecordAProtocolElement(pContext, client, XRecordFromClient, - (pointer)stuff, SIZEOF(xReq), bytesLeft); + (pointer)stuff, SIZEOF(xReq), 0, bytesLeft); /* reinsert the extended length field that was squished out */ bigLength = client->req_len + bytes_to_int32(sizeof(bigLength)); if (client->swapped) swapl(&bigLength); RecordAProtocolElement(pContext, client, XRecordFromClient, - (pointer)&bigLength, sizeof(bigLength), /* continuation */ -1); + (pointer)&bigLength, sizeof(bigLength), 0, /* continuation */ -1); bytesLeft -= sizeof(bigLength); /* record the rest of the request after the length */ RecordAProtocolElement(pContext, client, XRecordFromClient, - (pointer)(stuff + 1), bytesLeft, /* continuation */ -1); + (pointer)(stuff + 1), bytesLeft, 0, /* continuation */ -1); } /* RecordABigRequest */ @@ -542,7 +548,7 @@ RecordARequest(ClientPtr client) RecordABigRequest(pContext, client, stuff); else RecordAProtocolElement(pContext, client, XRecordFromClient, - (pointer)stuff, client->req_len << 2, 0); + (pointer)stuff, client->req_len << 2, 0, 0); } else /* extension, check minor opcode */ { @@ -566,7 +572,7 @@ RecordARequest(ClientPtr client) else RecordAProtocolElement(pContext, client, XRecordFromClient, (pointer)stuff, - client->req_len << 2, 0); + client->req_len << 2, 0, 0); break; } } /* end for each minor op info */ @@ -619,7 +625,8 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata) if (pContext->continuedReply) { RecordAProtocolElement(pContext, client, XRecordFromServer, - (pointer)pri->replyData, pri->dataLenBytes, /* continuation */ -1); + (pointer)pri->replyData, pri->dataLenBytes, + pri->padBytes, /* continuation */ -1); if (!pri->bytesRemaining) pContext->continuedReply = 0; } @@ -629,7 +636,7 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata) if (majorop <= 127) { /* core reply */ RecordAProtocolElement(pContext, client, XRecordFromServer, - (pointer)pri->replyData, pri->dataLenBytes, pri->bytesRemaining); + (pointer)pri->replyData, pri->dataLenBytes, 0, pri->bytesRemaining); if (pri->bytesRemaining) pContext->continuedReply = 1; } @@ -651,7 +658,7 @@ RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata) { RecordAProtocolElement(pContext, client, XRecordFromServer, (pointer)pri->replyData, - pri->dataLenBytes, pri->bytesRemaining); + pri->dataLenBytes, 0, pri->bytesRemaining); if (pri->bytesRemaining) pContext->continuedReply = 1; break; @@ -723,7 +730,7 @@ RecordADeliveredEventOrError(CallbackListPtr *pcbl, pointer nulldata, pointer ca } RecordAProtocolElement(pContext, pClient, - XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0); + XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0, 0); } } /* end for each event */ } /* end this client is on this context */ @@ -774,7 +781,7 @@ RecordSendProtocolEvents(RecordClientsAndProtocolPtr pRCAP, } RecordAProtocolElement(pContext, NULL, - XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0); + XRecordFromServer, pEvToRecord, SIZEOF(xEvent), 0, 0); /* make sure device events get flushed in the absence * of other client activity */ @@ -2415,7 +2422,7 @@ ProcRecordEnableContext(ClientPtr client) assert(numEnabledContexts > 0); /* send StartOfData */ - RecordAProtocolElement(pContext, NULL, XRecordStartOfData, NULL, 0, 0); + RecordAProtocolElement(pContext, NULL, XRecordStartOfData, NULL, 0, 0, 0); RecordFlushReplyBuffer(pContext, NULL, 0, NULL, 0); return Success; } /* ProcRecordEnableContext */ @@ -2446,7 +2453,7 @@ RecordDisableContext(RecordContextPtr pContext) if (!pContext->pRecordingClient) return; if (!pContext->pRecordingClient->clientGone) { - RecordAProtocolElement(pContext, NULL, XRecordEndOfData, NULL, 0, 0); + RecordAProtocolElement(pContext, NULL, XRecordEndOfData, NULL, 0, 0, 0); RecordFlushReplyBuffer(pContext, NULL, 0, NULL, 0); /* Re-enable request processing on this connection. */ AttendClient(pContext->pRecordingClient); @@ -2761,7 +2768,7 @@ RecordConnectionSetupInfo(RecordContextPtr pContext, NewClientInfoRec *pci) SwapConnSetupPrefix(pci->prefix, (xConnSetupPrefix*)pConnSetup); SwapConnSetupInfo((char*)pci->setup, (char*)(pConnSetup + prefixsize)); RecordAProtocolElement(pContext, pci->client, XRecordClientStarted, - (pointer)pConnSetup, prefixsize + restsize, 0); + (pointer)pConnSetup, prefixsize + restsize, 0, 0); free(pConnSetup); } else @@ -2770,9 +2777,9 @@ RecordConnectionSetupInfo(RecordContextPtr pContext, NewClientInfoRec *pci) * data in two pieces */ RecordAProtocolElement(pContext, pci->client, XRecordClientStarted, - (pointer)pci->prefix, prefixsize, restsize); + (pointer)pci->prefix, prefixsize, 0, restsize); RecordAProtocolElement(pContext, pci->client, XRecordClientStarted, - (pointer)pci->setup, restsize, /* continuation */ -1); + (pointer)pci->setup, restsize, 0, /* continuation */ -1); } } /* RecordConnectionSetupInfo */ @@ -2849,7 +2856,7 @@ RecordAClientStateChange(CallbackListPtr *pcbl, pointer nulldata, pointer callda { if (pContext->pRecordingClient && pRCAP->clientDied) RecordAProtocolElement(pContext, pClient, - XRecordClientDied, NULL, 0, 0); + XRecordClientDied, NULL, 0, 0, 0); RecordDeleteClientFromRCAP(pRCAP, pos); } } -- 1.7.1 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
