On Tue, Sep 13, 2011 at 07:31:12PM +0800, Sam Spilsbury wrote: > Hi, > > I haven't contributed any patches to X11 before so excuse me if I'm > terribly in the wrong. > > I noticed that the server is not removing the SendEvent (0x80) bit in > ProcSendEvent in dix/events.c before doing range checks on the event > type. This would cause these range checks to return an invalid BadValue > error. > > I am not sure if this is *really* a bug in the server or a bug in > extension libraries (like libXext) who set the SendEvent bit before > converting the event to wire format, since ProcSendEvent sets the > SendEvent bit anyways just before it writes the event to the client. > > This at least fixed a case for me where sending synthetic ShapeNotify > events to clients resulted in a BadValue error. > > Anyways, patch attached, > > Thanks, > > Sam
> From 4ce5d1cab01b911ab9672dbd1e773faace62e243 Mon Sep 17 00:00:00 2001 > From: Sam Spilsbury <[email protected]> > Date: Tue, 13 Sep 2011 19:17:56 +0800 > Subject: [PATCH] Fix SendEvent requests coming from extensions which set 0x80 > being invalid. > > Some (broken?) extension libraries set the SendEvent "magic" bit in the > event->type field before sending the request down the wire, so when we > did a range check on event->type it is possible that it could have been > invalid (this is at least the case for XShape). As such, we should remove 0x80 > from the bitfield before doing a range check on the event. This is safe > since we will re-set 0x80 on the bitfield after checking the event > before writing it to the client. > --- > dix/events.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/dix/events.c b/dix/events.c > index 8a4c6b9..cf24869 100644 > --- a/dix/events.c > +++ b/dix/events.c > @@ -5241,6 +5241,17 @@ ProcSendEvent(ClientPtr client) > > REQUEST_SIZE_MATCH(xSendEventReq); > > + /* libXext and other extension libraries may set the bit indicating > + * that this event came from a SendEvent request so remove it > + * since otherwise the event type may fail the range checks > + * and cause an invalid BadValue error to be returned. > + * > + * This is safe to do since we later add the SendEvent bit (0x80) > + * back in once we send the event to the client */ > + > + if (stuff->event.u.u.type & 0x80) > + stuff->event.u.u.type &= ~(0x80); > + I wouldn't mind if you added a define for this to make the code a bit more obvious. other than that, see jamey's comments. happy to merge that once addressed. Cheers, Peter > /* The client's event type must be a core event type or one defined by an > extension. */ > > -- > 1.7.5.4 > > _______________________________________________ > [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
