Yet another step of uncluttering includes: move out the BUG_* macros
into a separate header, which then is included as-needed.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The `XISendDeviceHierarchyEvent()` function allocates space to store up
to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.
If a device with a given ID was removed and a new device with the same
ID added both in the same operation, the single device ID will lead to
two info structures being written to `info`.
Since this case can occur for every device ID at once, a total of two
times `MAXDEVICES` info structures might be written to the allocation.
To avoid it, once one add/remove master is processed, send out the
device hierarchy event for the current state and continue. That event
thus only ever has exactly one of either added/removed in it (and
optionally slave attached/detached).
CVE-2024-21885, ZDI-CAN-22744
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
There's a racy sequence where a master device may copy the button class
from the slave, without ever initializing numButtons. This leads to a
device with zero buttons but a button class which is invalid.
Let's copy the numButtons value from the source - by definition if we
don't have a button class yet we do not have any other slave devices
with more than this number of buttons anyway.
CVE-2024-0229, ZDI-CAN-22678
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Both DeviceFocusEvent and the XIQueryPointer reply contain a bit for
each logical button currently down. Since buttons can be arbitrarily mapped
to anything up to 255 make sure we have enough bits for the maximum mapping.
CVE-2023-6816, ZDI-CAN-22664, ZDI-CAN-22665
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
If we remove a master device and specify which other master devices
attached slaves should be returned to, enforce that those two are
indeeed a pointer and a keyboard.
Otherwise we can try to attach the keyboards to pointers and vice versa,
leading to possible crashes later.
button->xkb_acts is supposed to be an array sufficiently large for all
our buttons, not just a single XkbActions struct. Allocating
insufficient memory here means when we memcpy() later in
XkbSetDeviceInfo we write into memory that wasn't ours to begin with,
leading to the usual security ooopsiedaisies.
CVE-2023-6377, ZDI-CAN-22412, ZDI-CAN-22413
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
The handling of appending/prepending properties was incorrect, with at
least two bugs: the property length was set to the length of the new
part only, i.e. appending or prepending N elements to a property with P
existing elements always resulted in the property having N elements
instead of N + P.
Second, when pre-pending a value to a property, the offset for the old
values was incorrect, leaving the new property with potentially
uninitalized values and/or resulting in OOB memory writes.
For example, prepending a 3 element value to a 5 element property would
result in this 8 value array:
[N, N, N, ?, ?, P, P, P ] P, P
^OOB write
The XI2 code is a copy/paste of the RandR code, so the bug exists in
both.
CVE-2023-5367, ZDI-CAN-22153
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
CVE-2023-0494, ZDI-CAN-19596
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This fixes an OOB read and the resulting information disclosure.
Length calculation for the request was clipped to a 32-bit integer. With
the correct stuff->num_items value the expected request size was
truncated, passing the REQUEST_FIXED_SIZE check.
The server then proceeded with reading at least stuff->num_items bytes
(depending on stuff->format) from the request and stuffing whatever it
finds into the property. In the process it would also allocate at least
stuff->num_items bytes, i.e. 4GB.
The same bug exists in ProcChangeProperty and ProcXChangeDeviceProperty,
so let's fix that too.
CVE-2022-46344, ZDI-CAN 19405
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
Both ProcXChangeDeviceProperty and ProcXIChangeProperty checked the
property for validity but didn't actually return the potential error.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
The XKB protocol effectively prevents us from ever using keycodes above
255. For buttons it's theoretically possible but realistically too niche
to worry about. For all other passive grabs, the detail must be zero
anyway.
This fixes an OOB write:
ProcXIPassiveUngrabDevice() calls DeletePassiveGrabFromList with a
temporary grab struct which contains tempGrab->detail.exact = stuff->detail.
For matching existing grabs, DeleteDetailFromMask is called with the
stuff->detail value. This function creates a new mask with the one bit
representing stuff->detail cleared.
However, the array size for the new mask is 8 * sizeof(CARD32) bits,
thus any detail above 255 results in an OOB array write.
CVE-2022-46341, ZDI-CAN 19381
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
When processing events we operate on InternalEvent pointers. They may
actually refer to a an instance of DeviceEvent, GestureEvent or any
other event that comprises the InternalEvent union. This works well in
practice because we always look into event type before doing anything,
except in the case of copying the event.
*dst_event = *src_event would copy whole InternalEvent event and would
cause out of bounds read in case the pointed to event was not
InternalEvent but e.g. DeviceEvent.
This regression has been introduced in
23a8b62d34.
Fixes https://gitlab.freedesktop.org/xorg/xserver/-/issues/1261
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
This fixes a crash when a DeviceEvent struct converted to
InteralEvent was beeing copied as InternalEvent (and thus
causing out of bounds reads) in ActivateGrabNoDelivery()
in events.c: 3876 *grabinfo->sync.event = *real_event;
Possible fix for https://gitlab.freedesktop.org/xorg/xserver/-/issues/1253
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
[45/388] Compiling C object Xi/liblibxserver_xi.a.p/xichangehierarchy.c.o
../Xi/xichangehierarchy.c:61:32: warning: argument 1 of type ‘int[256]’ with mismatched bound [-Warray-parameter=]
61 | XISendDeviceHierarchyEvent(int flags[MAXDEVICES])
| ~~~~^~~~~~~~~~~~~~~~~
In file included from ../Xi/xichangehierarchy.c:54:
../Xi/xichangehierarchy.h:42:37: note: previously declared as ‘int[]’
42 | void XISendDeviceHierarchyEvent(int flags[]);
| ~~~~^~~~~~~
libxcb 14.1 and older are not forwards-compatible with new device
classes as it does not properly ignore unknown device classes. Since
breaking libxcb would break quite a lot of applications, we instead
report Gesture device class only if the client advertised support for XI
2.4.
Clients may still not work in cases when a client advertises XI 2.4
support and then a completely separate module within the client uses
broken libxcb to call XIQueryDevice.
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
CVE-2021-3472 / ZDI-CAN-1259
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Delivery of emulated events usually happens only to the owning client.
If there are grabs, only the grabbing client may receive these events.
This logic does not work during the touch event replay in
DeactivatePointerGrab(), as the previous grab is no longer in the
listener queue of the touch, so the next owner gets whole emulated event
sequence. This may trigger implicit grabs. After replay,
DeactivatePointerGrab() will update the global grab without regard to
this new implicit grab, which leads to issues down the line.
This change is effectively the same as 35e5a76cc1 except that the change
is limited to only emulated pointer events. Otherwise, in the case of a
device grab we end up not sending any touch events to clients that
selected XI_TouchOwnership event and should get touch events before they
get ownership of touch sequence.
Fixes#7https://bugs.freedesktop.org/show_bug.cgi?id=96536
Pointer-emulated touch events should only be delivered to the client
that owns the sequence even if it's a core client that became the
effective owner of the sequency by selecting for pointer press and
movement.
Currently the emulated events are delivered like this already (see
TouchResourceIsOwner() check in DeliverEmulatedMotionEvent()), except in
the case of TouchEnd, in which case the generated motion event is still
delivered to some client that's not necessarily the owner of the touch
sequence.
We already know whether a touch sequence that is about to emulate a
pointer event has an owner, we just need to check that. This further
allows to simplify DeliverEmulatedMotionEvent() as it won't ever be
called for non-owned touch events.
https://bugs.freedesktop.org/show_bug.cgi?id=60394
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
CVE-2020-14346 / ZDI-CAN-11429
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Most (but not all) of these were found by using
codespell --builtin clear,rare,usage,informal,code,names
but not everything reported by that was fixed.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
We can't have high keycodes because everything in XKB relies on 8 bits. XI2's
API allows for 32-bit keycodes so we have to take those but nothing in the
server is really ready for this. The effect of this right now is that any high
keycode grab is clipped to 255 and thus ends up grabbing a different key
instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1697804
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The input thread checks the barriers for pointer positioning, swapping the
list out from underneath is considered impolite.
Reported-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
When Retrieving touch delivery data we need to check if we have an active
grab on such device, and in that case use it to delivery events.
If we don't do this, when rejecting the touch events in DeactivatePointerGrab,
we will end-up in creating an implicit grab that will change the device
deviceGrab's state, causing a recursion during TouchEndTouch.
Fixes#7https://bugs.freedesktop.org/show_bug.cgi?id=96536
They were defined as empty macros on all platforms except for the
long unsupported Cray systems which needed to use bitfields to define
types smaller than 64-bits.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
This has caused nonsensical values in xinput output.
Signed-off-by: Roman Kapl <code@rkapl.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The byte-swapping code forgot that the xXIButtonInfo is followed by a
button mask, not directly by the button labels. This resulted in client
crashes in cross-endian setups, for example in `xinput list --long`,
since the client got an invalid atom.
A new function was introduced to get the right positions for the label
and mask data.
Signed-off-by: Roman Kapl <code@rkapl.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Roundhouse kick replacing the various (sizeof(foo)/sizeof(foo[0])) with
the ARRAY_SIZE macro from dix.h when possible. A semantic patch for
coccinelle has been used first. Additionally, a few macros have been
inlined as they had only one or two users.
Signed-off-by: Daniel Martin <consume.noise@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
[jcristau: originally this patch fixed the same issue as commit
211e05ac85 "Xi: Test exact size of XIBarrierReleasePointer", with the
addition of these checks]
This addresses CVE-2017-12179
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Signed-off-by: Nathan Kidd <nkidd@opentext.com>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Otherwise a client can send any value of num_barriers and cause reading or swapping of values on heap behind the receive buffer.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The SProcXSendExtensionEvent must not attempt to swap GenericEvent because
it is assuming that the event has fixed size and gives the swapping function
xEvent-sized buffer.
A GenericEvent would be later rejected by ProcXSendExtensionEvent anyway.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The requirement is that events have type in range
EXTENSION_EVENT_BASE..lastEvent, but it was tested
only for first event of all.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Make sure that the xEvent eventT is initialized with zeros, the same way as
in SProcSendEvent.
Some event swapping functions do not overwrite all 32 bytes of xEvent
structure, for example XSecurityAuthorizationRevoked. Two cooperating
clients, one swapped and the other not, can send
XSecurityAuthorizationRevoked event to each other to retrieve old stack data
from X server. This can be potentialy misused to go around ASLR or
stack-protector.
Signed-off-by: Michal Srb <msrb@suse.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Just like we do with XWarpPointer's.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This is a work in progress that builds Xvfb, Xephyr, Xwayland, Xnest,
and Xdmx so far. The outline of Xquartz/Xwin support is in tree, but
hasn't been built yet. The unit tests are also not done.
The intent is to build this as a complete replacement for the
autotools system, then eventually replace autotools. meson is faster
to generate the build, faster to run the bulid, shorter to write the
build files in, and less error-prone than autotools.
v2: Fix indentation nits, move version declaration to project(), use
existing meson_options for version-config.h's vendor name/web.
Signed-off-by: Eric Anholt <eric@anholt.net>
Acked-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
This touches everything that ends up in the Xorg binary; the big missing
part is GLX since that's all generated code. Cuts about 14k from the
binary on amd64.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
The previous code only worked when the barrier was created by the same client
as the one calling XIChangeDeviceHierarchy.
http://bugzilla.redhat.com/show_bug.cgi?id=1384432
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Commits 816015648f and
fee0827a9a made it so that
wl_keyboard::enter doesn't result in X clients getting KeyPress events
while still updating our internal xkb state to be in sync with the
host compositor.
wl_keyboard::leave needs to be handled in the same way as its
semantics from an X client POV should be the same as an X grab getting
triggered, i.e. X clients shouldn't get KeyRelease events for keys
that are still down at that point.
This patch uses LeaveNotify for these events on wl_keyboard::leave and
changes the current use of KeymapNotify to EnterNotify instead just to
keep some symmetry between both cases.
On ProcessDeviceEvent() we still need to deactivate X grabs if needed
for KeyReleases.
Signed-off-by: Rui Matos <tiagomatos@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Special case for the systemd-logind case in xfree86: when we're vt-switched
away and a device is plugged in, we get a paused fd from logind. Since we
can't probe the device or do anything with it, we store that device in the
xfree86 and handle it later when we vt-switch back. The device is not added to
inputInfo.devices until that time.
When the device is removed while still vt-switched away, the the config system
never notifies the DDX. It only runs through inputInfo.devices and our device
was never added to that.
When a device is plugged in, removed, and plugged in again while vt-switched
away, we have two entries in the xfree86-specific list that refer to the same
device node, both pending for addition later. On VT switch back, the first one
(the already removed one) will be added successfully, the second one (the
still plugged-in one) fails. Since the fd is correct, the device works until
it is removed again. The removed devices' config_info (i.e. the syspath)
doesn't match the actual device we addded tough (the input number increases
with each plug), it doesn't get removed, the fd remains open and we lose track
of the fd count. Plugging the device in again leads to a dead device.
Fix this by adding a call to notify the DDX to purge any remainders of devices
with the given config_info, that's the only identifiable bit we have at this
point.
https://bugs.freedesktop.org/show_bug.cgi?id=97928
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
This keeps the input driver SetProperty function from being called
while input events are being processed.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This removes all of the SIGIO handling support used for input
throughout the X server, preparing the way for using threads for input
handling instead.
Places calling OsBlockSIGIO and OsReleaseSIGIO are marked with calls
to stub functions input_lock/input_unlock so that we don't lose this
information.
xfree86 SIGIO support is reworked to use internal versions of
OsBlockSIGIO and OsReleaseSIGIO.
v2: Don't change locking order (Peter Hutterer)
v3: Comment weird && FALSE in xf86Helper.c
Leave errno save/restore in xf86ReadInput
Squash with stub adding patch (Peter Hutterer)
v4: Leave UseSIGIO config parameter so that
existing config files don't break (Peter Hutterer)
v5: Split a couple of independent patch bits out
of kinput.c (Peter Hutterer)
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
This was added in:
commit 312910b4e3
Author: Chase Douglas <chase.douglas@canonical.com>
Date: Wed Apr 18 11:15:40 2012 -0700
Update currentTime in dispatch loop
Unfortunately this is equivalent to calling GetTimeInMillis() once per
request. In the absolute best case (as on Linux) you're only hitting the
vDSO; on other platforms that's a syscall. Either way it puts a pretty
hard ceiling on request throughput.
Instead, push the call down to the requests that need it; basically,
grab processing and event generation.
Cc: Chase Douglas <chase.douglas@canonical.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
The touchpoint knows whether it should be emulating or not and we have a check
for that later. Check for this before we generate the event and try to deliver
it, lest we trigger a bug warning.
https://bugzilla.redhat.com/show_bug.cgi?id=1282252
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Add a new event source type for keypress events synthesised from focus
notifications (e.g. KeymapNotify from the parent server, when running
nested). This is used to keep the keys-down array in sync with the host
server's, without sending actual keypress events to clients.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
xichangehierarchy.c:424:23: warning: comparison of constant 536870911 with expression of type 'uint16_t'
(aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare,Semantic Issue]
if (stuff->length > (INT_MAX >> 2))
~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
xichangehierarchy.c:438:26: warning: comparison of constant 536870911 with expression of type 'uint16_t'
(aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare,Semantic Issue]
if ((any->length > (INT_MAX >> 2)) || (len < (any->length << 2)))
~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The client window can be closed in the middle of a touch sequence,
e.g. Qt 4 closes popup windows on MousePress and Qt 5.5 will do it
on TouchBegin. In this case the state of mouse buttons will not be
updated on TouchEnd because ProcessTouchEvent() calls UpdateDeviceState()
only when the event has been sent to the client. It results in a
stuck left mouse button.
This patch leads to calling UpdateDeviceState() in case the client
can't be found.
Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
While it's documented in the XACE spec, the XACE_KEY_AVAIL hook is
currently never actually invoked by the xserver.
This hook was added in 13c6713c82 (25 Aug 2006), but as the keyboard
processing was moved into XKB, the hook was forgotten and silently
dropped. The code calling this hook was removed by 7af53799c (4 Jan
2009), but it was probably already unused before that.
This patch re-adds support for this hook. The "count" hook parameter is
unused.
Signed-off-by: Andrew Eikum <aeikum@codeweavers.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
It's going to multiply anyway, so if we have non-constant values, might
as well let it do the multiplication instead of adding another multiply,
and good versions of calloc will check for & avoid overflow in the process.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Multiple functions in the Xinput extension handling of requests from
clients failed to check that the length of the request sent by the
client was large enough to perform all the required operations and
thus could read or write to memory outside the bounds of the request
buffer.
This commit includes the creation of a new REQUEST_AT_LEAST_EXTRA_SIZE
macro in include/dix.h for the common case of needing to ensure a
request is large enough to include both the request itself and a
minimum amount of extra data following the request header.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
I've been seeing sporadic (anywhere from once every few days to 3-4
times a day) crashes and freezes in X. The problematic behaviour isn't
always the same, but I chose a particular incident to debug, and found
that X was segfaulting in updateMotionHistory, on line 575 of
dix/getevents.c.
After some further investigation, I found that the bug was being
triggered when a SIGIO was received in DeepCopyPointerClasses, between
the AllocValuatorClass call (line 540) and updating the to->valuator
pointer (line 545). AllocValuatorClass calls realloc() on to->valuator,
so between these lines, it's not guaranteed to point to allocated
memory.
It seems the SIGIO handler is calling updateMotionHistory, which is
reading the memory pointed to by to->valuator and getting a wrong value
for last_motion, which updates buff to point to wildly the wrong place
and thus generates a segfault when a memcpy() is done into buff.
I am attaching a patch which I've been running on that machine for the
past three days, and haven't yet observed any more crashing or freezing
behaviour. The patch simply calls OsBlockSIGIO while
DeepCopyDeviceClasses is in progress, as the state of the X server's
device data structures is not guaranteed to be in a consistent state
during that time.
Debian bug#744303 <https://bugs.debian.org/744303>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
==26141== Invalid read of size 8
==26141== at 0x58FAEA: DeliverEmulatedMotionEvent (exevents.c:1484)
An InternalEvent is bigger than a DeviceEvent, thus copying one to the other
reads past the allocated boundary. Shouldn't have any real effect since we
shouldn't access anything past the DeviceEvent boundary if the event type is
correct.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This will also make it useful for cases when we have a new keymap to
apply to a device but don't have a source device.
Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The other values are checked correctly, but if a modifier was outside the
allowed range, it would go unnoticed and cause a out-of-bounds read error for
any mask equal or larger than 256. The DetailRec where we store the grab masks
is only sized to 8 * sizeof(Mask).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Fallout from fecc7eb1cf, and reverts most of the
rest of that patch.
The device name is allocated and may even change during PreInit. The const
warnings came from the test codes, the correct fix here is to fix the test
code.
touch.c: In function ‘touch_init’:
touch.c:254:14: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default]
dev.name = "test device";
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The request is followed by mask_len 4-byte units, then followed by the actual
modifiers.
Also fix up the swapping test, which had the same issue.
Reported-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
There's no reason for XI to declare 'typedef char *Pointer' in a
shared header file; assume it will eventually go away and stop using
it here.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
This lets us stop using the 'pointer' typedef in Xdefs.h as 'pointer'
is used throughout the X server for other things, and having duplicate
names generates compiler warnings.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Lots more const char stuff.
Remove duplicate defs of CoreKeyboardProc and CorePointerProc from
test/xi2/protocol-common.c
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
When a grab on a slave device is deactivated, the master device must
be checked, just in case there were events from other devices while
the slave device was stolen away by the passive grab. This may
introduce misbehaviors on mismatching valuators and device features
later on UpdateDeviceState().
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If an touch triggers an async button grab and that grab does not have the
ButtonReleaseMask set, the TouchEnd is never delivered, deliveries is 0 and
the grab is never deactivated.
If the grab is pointer async and keyboard sync, the keyboard events are stuck
in EnqueueEvent until some other pointer event terminates the grab.
Change this to check for the number of listeners. If we're about to deliver a
TouchEnd to a passive pointer grab, the number of listeners is already 1 -
pointer grabs always accept so other listeners were removed.
X.Org Bug 71878 <http://bugs.freedesktop.org/show_bug.cgi?id=71878>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Bug #71878 describes a bug resulting in the server ceasing to respond to
keyboard input after a touch event. The problem might be the following:
DeliverTouchBeginEvent tries to deliver an event to a listener of type
LISTENER_POINTER_REGULAR, taking the following if branch,
if (listener->type == LISTENER_POINTER_REGULAR ||
listener->type == LISTENER_POINTER_GRAB) {
rc = DeliverTouchEmulatedEvent(dev, ti, ev, listener, client, win,
grab, xi2mask);
if (rc == Success) {
listener->state = LISTENER_IS_OWNER;
/* async grabs cannot replay, so automatically accept this touch */
if (dev->deviceGrab.grab &&
dev->deviceGrab.fromPassiveGrab &&
dev->deviceGrab.grab->pointerMode == GrabModeAsync)
ActivateEarlyAccept(dev, ti);
}
goto out;
}
DeliverTouchEmulatedEvent succeeds. The deviceGrab meets all
three of the conditions of the inner if, enters
ActivateEarlyAccept which then fails due to,
BUG_RETURN(ti->listeners[0].type != LISTENER_GRAB &&
ti->listeners[0].type != LISTENER_POINTER_GRAB);
That is, despite listener->type == LISTENER_POINTER_REGULAR. With my
non-existent knowledge of XINPUT, it seems like the solution here
might be to only ActivateEarlyAccept when listener->type ==
LISTENER_POINTER_GRAB.
Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
gcc -Wlogical-op
exevents.c: In function 'DeliverEmulatedMotionEvent':
exevents.c:1480:13: warning: logical 'or' of collectively exhaustive
tests is always true [-Wlogical-op]
The relevant snippet of exevents.c:
1479 if (ti->listeners[0].type != LISTENER_POINTER_REGULAR ||
1480 ti->listeners[0].type != LISTENER_POINTER_GRAB)
1481 return;
This condition was always true, causing dropped motion events.
Reported-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
XIAllowEvents changed length in XI 2.2 (for the touchid). A bug in libXi
causes libXi to always use the new request length if the server supports
2.2, regardless of the client's XIQueryVersion request.
The server takes the client's XIQueryVersion request into account though,
resulting in a BadLength error if a 2.[0,1] client calls XIAllowEvents on a
XI 2.2+ server.
Can't fix this in libXi, so work around this in the server.
X.Org Bug 68554 <http://bugs.freedesktop.org/show_bug.cgi?id=68554>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Do not allow setting client version to an arbitrary value >= XIVersion.
Fixes a test error with test/xi2/protocol-xiqueryversion.c, introduced by
commit 4360514d1c "Xi: Allow clients to ask for 2.3 and then 2.2 without failing"
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This allows different sub-systems within the same application to
request different Xi versions without either getting old behaviour
everywhere or simply failing with a BadValue.
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Ungrabbing a device during an active touch grab rejects the grab. Ungrabbing
a device during an active pointer grab accepts the grab.
Rejection is not really an option for a pointer-emulated grab, if a client
has a button mask on the window it would get a ButtonPress emulated after
UngrabDevice. That is against the core grab behaviour.
X.Org Bug 66720 <http://bugs.freedesktop.org/show_bug.cgi?id=66720>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>
If we have one listener left but it's not a grab, it cannot be in
LISTENER_HAS_ACCEPTED state.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Introduced in xorg-server-1.13.99.901-2-g9ad0fdb. Storing the grab pointer
in the listener turns out to be a bad idea. If the grab is not an active
grab or an implicit grab, the pointer stored is the one to the grab attached
on the window. This grab may be removed if the client calls UngrabButton or
similar while the touch is still active, leaving a dangling pointer.
To avoid this, copy the grab wherever we need to reference it later. This
is also what we do for pointer/keyboard grabs, where we copy the grab as
soon as it becomes active.
Reported-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
EmitTouchEnd calls DeliverTouchEvents directly instead of through
public.processInputProc. If a device is frozen, the TouchEnd is
processed while the device is waiting for a XAllowEvents and thus ends the
touch point (and the grab) before the client decided what to do with it. In
the case of ReplayPointer, this loses the event.
This is a hack, but making EmitTouchEnd use processInputProc breaks
approximately everything, especially the touch point is cleaned up during
ProcessTouchEvents. Working around that is a bigger hack than this.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a device is frozen in results to a grab, we need to enqueue the events.
This makes things complicated, and hard to follow since touch events are now
replayed in the history, pushed into EnqueueEvent, then replayed later
during PlayReleasedEvents in response to an XAllowEvents.
While the device is frozen, no touch events are processed, so if there is a
touch client with ownership mask _below_ the grab this will delay the
delivery and potentially screw gesture recognition. However, this is the
behaviour we have already anyway if the top-most client is a sync pgrab or
there is a sync grab active on the device when the TouchBegin was generated.
(also note, such a client would only reliably work in case of ReplayPointer
anyway)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
If a touch is pending_finish and we just punted it to the next owner, that
client must receive a TouchEnd event.
If we just punted to the last owner and that owner not a touch grab, we need
to end the touch since this is the last event to be sent, and the client
cannot accept/reject this.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Delivering an event changes the state to LISTENER_IS_OWNER and we thus lose
the information of early acceptance.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Enrico Weigelt
Peter Hutterer
Povilas Kanapickas
Matthieu Herrb
Adam Jackson
Alan Coopersmith
Marco Trevisan (Treviño)
Dave Airlie
Roman Kapl
Daniel Martin
Nathan Kidd
Michal Srb
Carlos Garnacho
Eric Anholt
Rui Matos
Keith Packard
Daniel Stone
Jeremy Huddleston Sequoia
Alexander Volkov
Andrew Eikum
Olivier Fourdan
Steven McDonald
Jon TURNEY
Ben Gamari
Maarten Lankhorst