Bug 3722 - RECORD extension generates X protocol violation
Summary: RECORD extension generates X protocol violation
Status: RESOLVED MOVED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: 6.8.2
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-07 09:26 UTC by Daniel Stone
Modified: 2018-12-17 17:19 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:26:58 UTC
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.
Comment 1 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:31:26 UTC
Created attachment 3037 [details] [review]
A (dubious?) patch to fix the leak.
Comment 2 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:33:00 UTC
Created attachment 3038 [details] [review]
evdev-repeat-sanely-1.patch
Comment 3 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:33:48 UTC
Created attachment 3039 [details] [review]
evdev-xkb-2.patch
Comment 4 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:40:09 UTC
Created attachment 3040 [details]
a screenshot without patch
Comment 5 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-07 09:44:02 UTC
Created attachment 3041 [details]
a screenshot with patch
Comment 6 FreeDesktop Bugzilla Database Corruption Fix User 2005-07-08 10:12:18 UTC
(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.
Comment 7 Alan Coopersmith 2005-10-09 00:17:30 UTC
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.    
     
Comment 8 Alan Coopersmith 2005-10-09 00:20:05 UTC
A testcase and patch were also previously attached and since lost in bugzilla
disk death.
Comment 9 Daniel Stone 2007-02-27 01:27:17 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 10 Daniel Stone 2007-03-07 11:44:26 UTC
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!


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.