Bug 3561 - RECORD extension causes X server crash
Summary: RECORD extension causes X server crash
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: 6.8.2
Hardware: x86 (IA32) Linux (All)
: high critical
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 5041
  Show dependency treegraph
 
Reported: 2005-06-16 21:49 UTC by Daniel Stone
Modified: 2006-05-10 08:44 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description FreeDesktop Bugzilla Database Corruption Fix User 2005-06-16 21:49:16 UTC
hello.

When I use RECORD extension, my X server crashes.
This is the bug of RECORD extension and I fixed it.

file : xc/programs/Xserver/record/record.c
function : RecordADeliveredEventOrError()

static void
RecordADeliveredEventOrError(.......)
                  :
{
                  :
     if (pRCAP && (pRCAP->pDeliveredEventSet || pRCAP->pErrorSet))
     {
                  :
          if (pev->u.u.type == X_Error)
          {
               recordit = RecordIsMemberOfSet(pRCAP->pErrroSet,
                                           ((xError *)(pev))->errorCode);
          }
          else
          {
               recordit = RecordIsMemberOfSet(pRCAP->pDeliveredEventSet,
                                           pev->u.u.type & 0177);
          }
                  :
     }
                  :
}

I select "errors" to record, and NOT select "delivered_event" to record.
When "delivered_event" happens, X server crashes because the function
RecordIsMemberOfSet() attempts to read null pointer, pRCAP->pDeliveredEventSet.

(pRCAP->pDeliveredEventSet is null because i don't select to record it.)

two lines calling RecordIsMemberOfSet() should be changed, for example;

          if (pRCAP->pErrorSet)
               recordit = RecordIsMemberOfSet (pRCAP->pErrroSet,
                                           ((xError *)(pev))->errorCode);

                and

          if (pRCAP->pDeliveredEventSet)
               recordit = RecordIsMemberOfSet (pRCAP->pDeliveredEventSet,
                                           pev->u.u.type & 0177);


bye.
Comment 1 Adam Jackson 2005-06-27 15:31:36 UTC
Created attachment 2981 [details]
Output of glxinfo with Composite disabled
Comment 2 Paul Anderson 2005-06-28 14:04:25 UTC
(In reply to comment #1)
> Created an attachment (id=2981) [edit]
> record-crash-fix-1.patch
> 

Should the check for (pev->u.u.type == X_Error) be preserved when
checking if pRCAP->pErrorSet is non-NULL?  Otherwise, we could have
a case where the u.u.detail field of the event (which becomes
the errorCode field when casting to xError) could match in the
RecordIsMemberOfSet() call when we might not really want it to.  
I haven't tested this out, but it might be worth a look.  
Comment 3 FreeDesktop Bugzilla Database Corruption Fix User 2005-06-28 19:26:41 UTC
I tested, Mr.Anderson is right.
This is my testcase.

	delivered_events.first = 2
	delivered_events.last  = LASTEvent

	errors.first = BadDrawable (9)
	errors.last  = BadDrawable (9)

result: no delivered events (except u.u.detail == 9) are recorded.

I think correct fix is;

	if (pev->u.u.type == X_Error) 
	{ 
		if (pRCAP->pErrorSet) 
		{ 
		recordit = RecordIsMemberOfSet(pRCAP->pErrorSet, 
                                                ((xError *)(pev))->errorCode); 
		} 
	} 
	else if (pRCAP->pDeliveredEventSet) 
	{ 
		recordit = RecordIsMemberOfSet(pRCAP->pDeliveredEventSet, 
                                                   pev->u.u.type & 0177); 
	} 


bye.
Comment 4 Adam Jackson 2005-07-03 13:13:47 UTC
paul, thanks for the confirmation.  i'm not familiar with the Record code at all
so i was kinda guessing.

adding to the 7.0 tracker.
Comment 5 Alan Coopersmith 2005-10-04 00:26:44 UTC
Original bug submitter e-mail and comments lost in bugzilla disk death.
xorg-team archives show:

           Summary: RECORD extension causes X server crash
           Product: xorg
           Version: 6.8.2
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: critical
          Priority: P2
         Component: Server/general
        AssignedTo: xorg-team at lists.x.org
        ReportedBy: karakama-akira at naka.hitachi-hitec.com


hello.

When I use RECORD extension, my X server crashes.
This is the bug of RECORD extension and I fixed it.

file : xc/programs/Xserver/record/record.c
function : RecordADeliveredEventOrError()

static void
RecordADeliveredEventOrError(.......)
                  :
{
                  :
     if (pRCAP && (pRCAP->pDeliveredEventSet || pRCAP->pErrorSet))
     {
                  :
          if (pev->u.u.type == X_Error)
          {
               recordit = RecordIsMemberOfSet(pRCAP->pErrroSet,
                                           ((xError *)(pev))->errorCode);
          }
          else
          {
               recordit = RecordIsMemberOfSet(pRCAP->pDeliveredEventSet,
                                           pev->u.u.type & 0177);
          }
                  :
     }
                  :
}

I select "errors" to record, and NOT select "delivered_event" to record.
When "delivered_event" happens, X server crashes because the function
RecordIsMemberOfSet() attempts to read null pointer, pRCAP->pDeliveredEventSet.

(pRCAP->pDeliveredEventSet is null because i don't select to record it.)

two lines calling RecordIsMemberOfSet() should be changed, for example;

          if (pRCAP->pErrorSet)
               recordit = RecordIsMemberOfSet (pRCAP->pErrroSet,
                                           ((xError *)(pev))->errorCode);

                and

          if (pRCAP->pDeliveredEventSet)
               recordit = RecordIsMemberOfSet (pRCAP->pDeliveredEventSet,
                                           pev->u.u.type & 0177);


bye.          
Comment 6 Alan Coopersmith 2005-10-04 00:28:06 UTC
Ajax's patch lost in bugzilla disk death too.
Comment 7 Paul Anderson 2005-11-11 13:27:03 UTC
I don't have a "real" patch put together, but I think the discussion
before was along the lines of something like:

RecordADeliveredEventOrError()
{
....

    for (eci = 0; eci < numEnabledContexts; eci++)
    {
        ...
        if (pRCAP && (pRCAP->pDeliveredEventSet || pRCAP->pErrorSet))
        {
            ...
            for (ev = 0; ev < pei->count; ev++, pev++)
            {
                int recordit = 0;
                if ((pev->u.u.type == X_Error) && pRCAP->pErrorSet)
                {
                    recordit = RecordIsMemberOfSet(pRCAP->pErrorSet,
                                                ((xError *)(pev))->errorCode);
                }
                else if (pRCAP->pDeliveredEventSet)
                {
                    recordit = RecordIsMemberOfSet(pRCAP->pDeliveredEventSet,
                                                   pev->u.u.type & 0177);
                }
                if (recordit)
                {
                    ...
                }
            }
        }
    }
}
Comment 8 Erik Andren 2006-04-02 00:57:03 UTC
So, is this bug still an issue today or should it be closed?
Comment 9 Adam Jackson 2006-05-11 01:44:37 UTC
found the old patch, applied to 1.1 branch and head.


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.