Michel Dänzer <[email protected]> writes: > Without this change, there's no way for a client to explicitly change > or destroy an existing event mask entry. Trying to do so as specified > above would actually result in a new entry being created with > the mask value passed in.
Yup, this looks like a good thing to fix. Instead of walking the linked list, you should perform a resource lookup instead using the provided ID; that would be more consistent, and would let you catch errors where the provided resource ID was actually being used on a different window. If it does, you should return a Match error. You've also lost the call to LEGAL_NEW_RESOURCE in the path which creates a new event. > While we're at it, fix a memory leak if AddResource fails. You've made a very common mistake -- AddResource actually calls the delete function when it fails, so you don't need to free the object in the failure path. Here's an alternate (untested) patch which shows what I meant; use whatever bits of this seem useful.
From 9b5604ee4ea71576c16961af79df2a4cf487acd7 Mon Sep 17 00:00:00 2001 From: Keith Packard <[email protected]> Date: Thu, 28 Jul 2016 09:46:59 -0700 Subject: [PATCH xserver] present: Handle event mask updates as specified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the Present extension specification: If eventContext specifies an existing event context, then if eventMask is empty, PresentSelectInput deletes the specified context, otherwise the specified event context is changed to select a different set of events. If eventContext is an unused XID, then if eventMask is empty no operation is performed. Otherwise, a new event context is created selecting the specified events. Without this change, there's no way for a client to explicitly change or destroy an existing event mask entry. Trying to do so as specified above would actually result in a new entry being created with the mask value passed in. This bug was found by, and the commit log message written by Michel Dänzer <[email protected]> Cc: Michel Dänzer <[email protected]> Signed-off-by: Keith Packard <[email protected]> --- present/present_event.c | 29 ++++++++++++++++++++++++++--- present/present_request.c | 2 -- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/present/present_event.c b/present/present_event.c index c586c9a..0850445 100644 --- a/present/present_event.c +++ b/present/present_event.c @@ -208,14 +208,37 @@ present_send_idle_notify(WindowPtr window, CARD32 serial, PixmapPtr pixmap, stru int present_select_input(ClientPtr client, XID eid, WindowPtr window, CARD32 mask) { - present_window_priv_ptr window_priv = present_get_window_priv(window, mask != 0); + present_window_priv_ptr window_priv; present_event_ptr event; + int ret; - if (!window_priv) { + /* Check to see if we're modifying an existing event selection */ + ret = dixLookupResourceByType((void **) &event, eid, present_event_type, + client, DixWriteAccess); + if (ret == Success) { + /* Match error for the wrong window; also don't + * modify some other client's event selection + */ + if (event->window != window || event->client != client) + return BadMatch; if (mask) - return BadAlloc; + event->mask = mask; + else + FreeResource(eid, RT_NONE); return Success; } + if (ret != BadValue) + return ret; + + LEGAL_NEW_RESOURCE(eid, client); + + if (mask == 0) + return Success; + + window_priv = present_get_window_priv(window, TRUE); + + if (!window_priv) + return BadAlloc; event = calloc (1, sizeof (present_event_rec)); if (!event) diff --git a/present/present_request.c b/present/present_request.c index 35320b6..c7663fc 100644 --- a/present/present_request.c +++ b/present/present_request.c @@ -184,8 +184,6 @@ proc_present_select_input (ClientPtr client) REQUEST_SIZE_MATCH(xPresentSelectInputReq); - LEGAL_NEW_RESOURCE(stuff->eid, client); - rc = dixLookupWindow(&window, stuff->window, client, DixGetAttrAccess); if (rc != Success) return rc; -- 2.8.1
-- -keith
signature.asc
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
