[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>
