RECORD extension generates X protocol violation. This is the bug of RECORD's reply buffer management algorithm. RecordAProtocolElement (...) /* xc/programs/Xserver/record/record.c */ { : if (futurelen >= 0) { /* start of new protocol element */ : if (!pContext->numBufBytes) /* reply buffer is empty */ { create reply packet header } : /* adjust reply length in reply buffer */ : } /* end if not continued reply */ /* if space available >= space needed, buffer the data */ if (REPLY_BUF_SIZE - pContext->numBufBytes >= datalen + numElemHeaders) { : } else RecordFlushReplyBuffer (); } /* RecordAProtocolElement */ The point is: o RECORD's reply packet header is created if o recording a new protocol element (not a continued one) and o reply buffer is empty o recording element (both new one and continued one) is buffered if space available consider this scenario; step1: RECORD's reply buffer is empty. step2: X server calls WriteToClient() to send a part of reply (names "A1") step3: RecordAProtocolElement is called. it creates reply packet header in RECORD's reply buffer because the buffer is empty. then buffer "A1" with protocol element header. step4: X server calls WriteToClient() to send continued part of the same reply (names "A2") step5: RecordAProtocolElement is called again. reply buffer space is not enough to store "A2". so flush reply buffer (i.e. "A1") and send "A2". ("A2" is not buffered). now reply buffer is empty. step6: X server calls WriteToClient() to send the rest of the same reply (names "A3") step7: RecordAProtocolElement is called again. buffers "A3" really top of the reply buffer. no packet header is created because this reply is continued one. step8: X server calls WriteToClient() to send NEW reply (names "B") step9: RecordAProtocolElement is called again. append "B" to reply buffer with protocol element because this is a new reply. but reply packet header is not created because reply buffer is not empty. ...... So, recording client will receive protocol violated stream like below. [reply packet header]-[protocol element header]-[A1]-[A2]-[A3]- [protocol element header]-[B]. of course, expected stream is; [reply packet header]-[protocol element header]-[A1]-[A2]-[A3]- [reply packet header]-[protocol element header]-[B]. my solution is simple; not buffer continued reply if reply buffer is empty.
Created attachment 3037 [details] [review] A (dubious?) patch to fix the leak.
Created attachment 3038 [details] [review] evdev-repeat-sanely-1.patch
Created attachment 3039 [details] [review] evdev-xkb-2.patch
Created attachment 3040 [details] a screenshot without patch
Created attachment 3041 [details] a screenshot with patch
(In reply to comment #5) > Created an attachment (id=3041) [edit] > my fix of RECORD extension this attachment is not 6.8.2 but XFree86-4.3.0. 6.8.2 has same problem.
Original reporter e-mail and comments lost in bugzilla disk death. xorg-team archives show: Summary: RECORD extension generates X protocol violation Product: xorg Version: 6.8.2 Platform: PC OS/Version: Linux Status: NEW Severity: normal Priority: P2 Component: Server/general AssignedTo: xorg-team at lists.x.org ReportedBy: karakama-akira at naka.hitachi-hitec.com RECORD extension generates X protocol violation. This is the bug of RECORD's reply buffer management algorithm. RecordAProtocolElement (...) /* xc/programs/Xserver/record/record.c */ { : if (futurelen >= 0) { /* start of new protocol element */ : if (!pContext->numBufBytes) /* reply buffer is empty */ { create reply packet header } : /* adjust reply length in reply buffer */ : } /* end if not continued reply */ /* if space available >= space needed, buffer the data */ if (REPLY_BUF_SIZE - pContext->numBufBytes >= datalen + numElemHeaders) { : } else RecordFlushReplyBuffer (); } /* RecordAProtocolElement */ The point is: o RECORD's reply packet header is created if o recording a new protocol element (not a continued one) and o reply buffer is empty o recording element (both new one and continued one) is buffered if space available consider this scenario; step1: RECORD's reply buffer is empty. step2: X server calls WriteToClient() to send a part of reply (names "A1") step3: RecordAProtocolElement is called. it creates reply packet header in RECORD's reply buffer because the buffer is empty. then buffer "A1" with protocol element header. step4: X server calls WriteToClient() to send continued part of the same reply (names "A2") step5: RecordAProtocolElement is called again. reply buffer space is not enough to store "A2". so flush reply buffer (i.e. "A1") and send "A2". ("A2" is not buffered). now reply buffer is empty. step6: X server calls WriteToClient() to send the rest of the same reply (names "A3") step7: RecordAProtocolElement is called again. buffers "A3" really top of the reply buffer. no packet header is created because this reply is continued one. step8: X server calls WriteToClient() to send NEW reply (names "B") step9: RecordAProtocolElement is called again. append "B" to reply buffer with protocol element because this is a new reply. but reply packet header is not created because reply buffer is not empty. ...... So, recording client will receive protocol violated stream like below. [reply packet header]-[protocol element header]-[A1]-[A2]-[A3]- [protocol element header]-[B]. of course, expected stream is; [reply packet header]-[protocol element header]-[A1]-[A2]-[A3]- [reply packet header]-[protocol element header]-[B]. my solution is simple; not buffer continued reply if reply buffer is empty.
A testcase and patch were also previously attached and since lost in bugzilla disk death.
Sorry about the phenomenal bug spam, guys. Adding xorg-team@ to the QA contact so bugs don't get lost in future.
shame we lost the patch. of course, record's reply semantics also seem quite broken, given the crashes i've been seeing internally with our endurance testing tool. yay!
https://gitlab.freedesktop.org/xorg/xserver/issues/330
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.