This seems to have fallen off the radar. I've pulled it into my tree, so it'll eventually be pulled into master.
--Jeremy On Oct 4, 2011, at 2:25 AM, Rami Ylimäki wrote: > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
