Bug 3561

Created attachment 2981 [details]
Output of glxinfo with Composite disabled

(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.  

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.

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.

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.          

Ajax's patch lost in bugzilla disk death too.

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)
                {
                    ...
                }
            }
        }
    }
}

So, is this bug still an issue today or should it be closed?

found the old patch, applied to 1.1 branch and head.