When a device is removed while still frozen, the events queued for that
device remain while the device itself is freed.
As a result, replaying the events will cause a use after free.
To avoid the issue, make sure to dequeue and free any pending events on
a frozen device when removed.
CVE-2025-26600, ZDI-CAN-25871
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
If the compiler knows of a better algorithm for counting the number of
bits set in a word for the target CPU, let it use that, instead of the
classic algorithm optimized for PDP-6.
Based on xorg/lib/libxext@490a25e6f8a4d2482af4364c700b68ad11a4d10b
v2: make old version static inline, stop exporting after !1695
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1674>
* unexport functions from dixgrab.h, that aren't used by any driver/module.
* add paremeter names to prototypes
* add doxygen-style documentation for all the prototypes
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Allow NULL parameters to be passed to FreeGrab(), so callers don't all
need to check on their own anymore.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The dix-config.h include file is always present, so no need for
an extra check and conditional include.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The xnfreallocarray was added along (and just as an alias to) XNFreallocarray
back a decade ago. It's just used in a few places and it's only saves us from
passing the first parameter (NULL), so the actual benefit isn't really huge.
No (known) driver is using it, so the macro can be dropped entirely.
Fixes: ae75d50395
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1529>
The generic XaceHook() call isn't typesafe (und unnecessarily slow).
Better add an explicit function, just like we already have for others.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1556>
This breaks the xf86-input-synaptics driver:
synaptics.c: In function 'clickpad_guess_clickfingers':
synaptics.c:2638:5: error: implicit declaration of function 'BUG_RETURN_VAL' [-Werror=implicit-function-declaration]
2638 | BUG_RETURN_VAL(hw->num_mt_mask > sizeof(close_point) * 8, 0);
This reverts commit 442aec2219.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1316>
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>
Disabling a master device floats all slave devices but we didn't do this
to already-disabled slave devices. As a result those devices kept their
reference to the master device resulting in access to already freed
memory if the master device was removed before the corresponding slave
device.
And to match this behavior, also forcibly reset that pointer during
CloseDownDevices().
Related to CVE-2024-21886, ZDI-CAN-22840
The `DisableDevice()` function is called whenever an enabled device
is disabled and it moves the device from the `inputInfo.devices` linked
list to the `inputInfo.off_devices` linked list.
However, its link/unlink operation has an issue during the recursive
call to `DisableDevice()` due to the `prev` pointer pointing to a
removed device.
This issue leads to a length mismatch between the total number of
devices and the number of device in the list, leading to a heap
overflow and, possibly, to local privilege escalation.
Simplify the code that checked whether the device passed to
`DisableDevice()` was in `inputInfo.devices` or not and find the
previous device after the recursion.
CVE-2024-21886, ZDI-CAN-22840
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
XTest requests lets the client specify a device ID, only if none
is specified do we fall back to the XTEST special device.
As of commit
aa4074251 input: Add new hook DeviceSendEventsProc for XTEST
regular devices are no longer able to send XTest events because they
have no sendEventsProc set.
This caused issue #1574 and the crash was fixed with commit
e820030de xtest: Check whether there is a sendEventsProc to call
but we still cannot send XTest events through a specific device.
Fix this by defaulting every device to the XTest send function and
punting it to the DDX (i.e. Xwayland) to override the devices as
necessary.
Fixes e820030de2
Fixes aa4074251f
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
This is to make sure the hardware gets the device states regardless
whether the internal state has changed or not, to overcome situations
that device LEDs are out of sync e.g. switching between VTs.
Signed-off-by: Yao Wei (魏銘廷) <yao.wei@canonical.com>
The code in AttachDevice() may free the dev->spriteInfo->sprite under
some circumstances and later call GetCurrentRootWindow() which uses
the same dev->spriteInfo->sprite.
While it seems unlikely that this is actually an issue, considering the
cases where one or the other get called, it still makes the code look
suspicious.
Make sure to clear set dev->spriteInfo->sprite to NULL immediately
after it's freed to avoid any confusion, even if only to clarify the
code.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1436
This fixes access to freed heap memory via dev->master. E.g. when
running BarrierNotify.ReceivesNotifyEvents/7 test from
xorg-integration-tests:
==24736==ERROR: AddressSanitizer: heap-use-after-free on address
0x619000065020 at pc 0x55c450e2b9cf bp 0x7fffc532fd20 sp 0x7fffc532fd10
READ of size 4 at 0x619000065020 thread T0
#0 0x55c450e2b9ce in GetMaster ../../../dix/devices.c:2722
#1 0x55c450e9d035 in IsFloating ../../../dix/events.c:346
#2 0x55c4513209c6 in GetDeviceUse ../../../Xi/xiquerydevice.c:525
../../../Xi/xichangehierarchy.c:95
#4 0x55c450e3455c in RemoveDevice ../../../dix/devices.c:1204
../../../hw/xfree86/common/xf86Xinput.c:1142
#6 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
#7 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
#8 0x55c450e837ef in dix_main ../../../dix/main.c:302
#9 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
#11 0x55c450d0113d in _start (/usr/lib/xorg/Xorg+0x117713d)
0x619000065020 is located 160 bytes inside of 912-byte region
[0x619000064f80,0x619000065310)
freed by thread T0 here:
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10d7cf)
#1 0x55c450e19f1c in CloseDevice ../../../dix/devices.c:1014
#2 0x55c450e343a4 in RemoveDevice ../../../dix/devices.c:1186
../../../hw/xfree86/common/xf86Xinput.c:1142
#4 0x55c450e17b04 in CloseDeviceList ../../../dix/devices.c:1038
#5 0x55c450e1de85 in CloseDownDevices ../../../dix/devices.c:1068
#6 0x55c450e837ef in dix_main ../../../dix/main.c:302
#7 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
previously allocated by thread T0 here:
(/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6)
#1 0x55c450e1c57b in AddInputDevice ../../../dix/devices.c:259
#2 0x55c450e34840 in AllocDevicePair ../../../dix/devices.c:2755
#3 0x55c45130318f in add_master ../../../Xi/xichangehierarchy.c:152
../../../Xi/xichangehierarchy.c:465
#5 0x55c4512cb9f5 in ProcIDispatch ../../../Xi/extinit.c:390
#6 0x55c450e6a92b in Dispatch ../../../dix/dispatch.c:551
#7 0x55c450e834b7 in dix_main ../../../dix/main.c:272
#8 0x55c4517a8d93 in main ../../../dix/stubmain.c:34
(/lib/x86_64-linux-gnu/libc.so.6+0x28564)
The problem is caused by dev->master being not reset when disabling the
device, which then causes dangling pointer when the master device itself
is being deleted when exiting whole server.
Note that RecalculateMasterButtons() requires dev->master to be still
valid, so we can reset it only at the end of function.
Signed-off-by: Povilas Kanapickas <povilas@radix.lt>
Fix the following race:
Possible data race during read of size 8 at 0xA112510 by thread #6
Locks held: 1, at address 0x366B40
at 0x14C8B9: GetMaster (devices.c:2691)
by 0x15CFC5: IsFloating (events.c:346)
by 0x2B9554: miPointerGetScreen (mipointer.c:527)
by 0x1A5136: xf86PostButtonEventM (xf86Xinput.c:1379)
by 0x1A52BD: xf86PostButtonEvent (xf86Xinput.c:1345)
by 0x485F45B: EvdevProcessEvent (in /usr/lib64/xorg/modules/input/evdev_drv.so)
by 0x485FDAC: EvdevReadInput (in /usr/lib64/xorg/modules/input/evdev_drv.so)
by 0x195427: xf86ReadInput (xf86Events.c:247)
by 0x2CC113: InputReady (inputthread.c:180)
by 0x2CE4EA: ospoll_wait (ospoll.c:657)
by 0x2CC077: InputThreadDoWork (inputthread.c:369)
by 0x484A336: mythread_wrapper (hg_intercepts.c:406)
This conflicts with a previous write of size 8 by thread #1
Locks held: none
at 0x14D2C6: AttachDevice (devices.c:2609)
by 0x15CF85: ReattachToOldMaster (events.c:1457)
by 0x1647DD: DeactivateKeyboardGrab (events.c:1700)
by 0x25D7F1: ProcXIUngrabDevice (xigrabdev.c:169)
by 0x2552AD: ProcIDispatch (extinit.c:398)
by 0x155291: Dispatch (dispatch.c:479)
by 0x158CBA: dix_main (main.c:276)
by 0x143A3D: main (stubmain.c:34)
Address 0xa112510 is 336 bytes inside a block of size 904 alloc'd
at 0x4846571: calloc (vg_replace_malloc.c:1328)
by 0x14A0B3: AddInputDevice (devices.c:260)
by 0x1A31A0: xf86ActivateDevice (xf86Xinput.c:365)
by 0x1A4549: xf86NewInputDevice (xf86Xinput.c:948)
by 0x1A4B44: NewInputDeviceRequest (xf86Xinput.c:1090)
by 0x1B81FE: device_added (udev.c:282)
by 0x1B8516: config_udev_init (udev.c:439)
by 0x1B7091: config_init (config.c:50)
by 0x197970: InitInput (xf86Init.c:814)
by 0x158C6B: dix_main (main.c:250)
by 0x143A3D: main (stubmain.c:34)
Block was alloc'd by thread #1
The steps to trigger the race are:
1. Main thread does cleanup at mipointer.c:360 setting the slave device's
miPointerPtr to null.
2. Input thread use MIPOINTER in mipointer.c and get the slave's
miPointerPtr = null.
3. Main thread updates dev->master at devices.c:2609.
4. MIPOINTER would now return the master's miPointerPtr but the input
thread already got the slave's miPointerPtr in step 2 and segfaults by
null ptr deref.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1260
Signed-off-by: Thomas Lindroth <thomas.lindroth@gmail.com>
Trying to change the pointer control settings on a device without
PtrFeedbackPtr would be a bug and a crash in the Xserver.
Guard against that case by returning early with a BadImplementation
error, that might kill the X11 client but the Xserver would survive.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Related: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1137
Resolves warning from Oracle Parfait static analyser:
Warning: Impossible or redundant condition
Impossible or redundant condition [impossible-redundant-condition]:
Condition 'dev != NULL' of branch is determined by previous branch
at line 270 of dix/devices.c in function 'AddInputDevice'.
Condition 'dev != NULL' from this branch implies following branch is always true at line 262
Fixes: commit 493ad83323
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
There is a race when reseting the XServer that causes spriteInfo to be
NULL in GetPairedDevice resulting a segfault and subsequent crash. The
problem was noticed when opening a connection, creating master devices,
destroying master devices and closing the connection during testing.
Signed-off-by: Arthur Williams <taaparthur@gmail.com>
Coverity complains about a use after free in here after the
freeing, I can't follow the linked list so well, but whot
says the device can only be on one list once, so break should
fix it.
Signed-off-by: Dave Airlie <airlied@redhat.com>
PtrCtrl really makes sense for relative pointing device only, absolute
devices such as touch devices do not have any PtrCtrl set.
In some cases, if the client issues a XGetPointerControl() immediatlely
after a ChangeMasterDeviceClasses() copied the touch device to the VCP,
a NULL pointer dereference will occur leading to a crash of Xwayland.
Check whether the PtrCtrl is not NULL in ProcGetPointerControl() and
return the default control values otherwise, to avoid the NULL pointer
dereference.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1519533
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
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>
This ensures that the deviceProc is never called while the input
thread is processing data from the device.
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>
-Wlogical-op now tells us:
devices.c:1685:23: warning: logical ‘and’ of equal expressions
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Adam Jackson <ajax@redhat.com>
If a device does not have any valuators, it makes no sense to set the
device transformation. Return a BadMatch error to let the caller know
that they're trying something stupid.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
Olivier Fourdan
Enrico Weigelt, metux IT consult
Alan Coopersmith
Peter Hutterer
José Expósito
Yao Wei
Povilas Kanapickas
tholin
Adam Jackson
Arthur Williams
Dave Airlie
Jeremy Huddleston Sequoia
Keith Packard
Jason Gerecke