Don't rely on this file just being included indirectly by somebody else
just by accident.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Move functions/macros dealing with request parsing or reply assembly/write
out of the big dix_priv.h into their own headers. This new header will also
get more of those function/macros soon (yet still in the pipeline).
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Move the walking loops on Xinerama screens into lambda-esque macros:
the callers look quite like we've been using lambda functions and
closures, but actually are just fancy macro trickery.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
When iterating screen lists, consistently use the same variable name
`walkScreenIdx` for holding current screen index everywhere.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The macro will automatically return BadAlloc if the buffer is broken,
otherwise Success. Thus, we don't need extra prior rpcbuf check.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This callback only had been added for Xwayland, which is gone now,
so we don't need it anymore. For property filtering (eg. security
extensions), we have PropertyFilterCallback.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This hook allows extensions to intercept client requests for changing
window attributes. It can either change the parameters or skip the
entire call (eg. handle all itself) so just the hook provided result
code is returned to the client.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
a) no need to checking for win->optional == NULL before calling
MakeWindowOptional(), because it checks itself
(except some cases where it's presence has it's own semantics,
or prevent unnecessary allocations)
b) lots of call sites didn't check for allocation failure.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of highjacking core request handlers, use the recently introduced
DDX/driver API.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't have a standard protocol for enabling VRR yet, but some time ago an
ad-hoc had been made in the amdgpu driver (later also copied to modsetting),
which works by client setting the _VARIABLE_REFRESH window property.
The way it's currently done - driver is highjacking the X_ChangeProperty and
X_DeleteProperty request handlers - is pretty fragile, and is also a violation
of layers: drivers never should be twisted with core protocol details. (And in
the future, this should be done by some suitable extension).
Another problem is Xinerama: when it's enabled, this only works on the first
screen - the others won't ever see this signal, no matter on which one(s) the
Window is physically placed (for the wire protocol, all windows are on screen 0,
unless the client explicitly creates them on another one)
This commit adds a generic Screen proc for telling the DDX, whether the VRR mode
shall be changed (for now, it's only DISABLED and ENABLED). Drivers can hook into
here in order to receive this signal, w/o having to highjack any core request
handlers. Catching the property change is now entirely done in the DIX.
The (non-standard) status qou of (ab)using window properties is kept, but it's
now also easy to add a new signaling mechanism, in case a standard is agreed on.
Yet a quite naive implementation (eg. not acting on moving windows between screens),
but enough to fix the most pressing problems supporting extra screens in general,
as well as stopping the highjacking of core request handlers by drivers.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Only key difference that calloc(), in contrast to rellocarray(),
is zero-initializing. The overhead is hard to measure on today's
machines, and it's safer programming practise to always allocate
zero-initialized, so one can't forget to do it explicitly.
Cocci rule:
@@
expression COUNT;
expression LEN;
@@
- xallocarray(COUNT,LEN)
+ calloc(COUNT,LEN)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
* use their actual path instead of relying this to be in compiler's
include path list.
* no need to do it only conditionally, no #ifdef needed
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for using a complex callback machinery, if we just move the
little pieces of byte-swapping directly into the request handler.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for using a complex callback machinery, if we just move the
little pieces of byte-swapping directly into the request handler.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Using calloc() instead of malloc() as preventive measure, so there
never can be any hidden bugs or leaks due uninitialized memory.
The extra cost of using this compiler intrinsic should be practically
impossible to measure - in many cases a good compiler can even deduce
if certain areas really don't need to be zero'd (because they're written
to right after allocation) and create more efficient machine code.
The code pathes in question are pretty cold anyways, so it's probably
not worth even thinking about potential extra runtime costs.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The pointer to the window properties is currently inside the WindowOptional
structure, which may or may not exist at any given time. Thus, before accessing
those fields, at least need to check whether it exists, potentially need to
create it first.
Since a pointer is small (in relation to WindowRec) and windows having properties
is a pretty common, we can make our life much simpler here by moving the pointer
directly into WindowRec, so we don't need extra WindowOptionalRec allocation.
This also fixes an analyzer warning on potential NULL dereference issue:
| ../dix/property.c: In function ‘dixChangeWindowProperty’:
|../dix/property.c:343:37: warning: dereference of NULL ‘*pWin.optional’ [CWE-476] [-Wanalyzer-null-dereference]
| 343 | pProp->next = pWin->optional->userProps;
| | ~~~~~~~~~~~~~~^~~~~~~~~~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
* not needed by any drivers, so no need to keep it exported
* correct return type is `enum EventDeliveryState`
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>
No real harm, but clears warning from gcc 14.1:
../dix/property.c: In function ‘ProcListProperties’:
..//dix/property.c:605:27: warning: dereference of NULL ‘temppAtoms’
[CWE-476] [-Wanalyzer-null-dereference]
605 | *temppAtoms++ = pProp->propertyName;
| ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1673>
It shouldn't matter, since it would have a length of 0, but it
clears warnings from gcc 14.1:
../dix/property.c: In function ‘dixChangeWindowProperty’:
../dix/property.c:287:9: warning: use of possibly-NULL ‘data’ where
non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
287 | memcpy(data, value, totalSize);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../dix/property.c:324:13: warning: use of possibly-NULL ‘data’ where
non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
324 | memcpy(data, value, totalSize);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1673>
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>
It is copied using memcpy() and not modified so we can add const. This
fixes a -Wincompatible-pointer-types-discards-qualifiers compiler warning
that was failing a -Werror XVnc build for me.
Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
This will be used by in-server features that need to react to property
changes. The first one will be _XWAYLAND_ALLOW_COMMITS.
Signed-off-by: Adam Jackson <ajax@redhat.com>
[Pekka: add commit message body]
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Make sure the whole event is initialized, instead of leaving the pad
bytes unset.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
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>
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>
Don't need to pass an empty reply to NullPropertyReply, let it make
it's own. Move reply initialization code in remaining replies in
ProcGetProperty to group with the rest of the fields. (Prepares
for coming C99 designated initializer conversion.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Tested-by: Daniel Stone <daniel@fooishbar.org>
This is strictly the application of the script 'x-indent-all.sh'
from util/modular. Compared to the patch that Daniel posted in
January, I've added a few indent flags:
-bap
-psl
-T PrivatePtr
-T pmWait
-T _XFUNCPROTOBEGIN
-T _XFUNCPROTOEND
-T _X_EXPORT
The typedefs were needed to make the output of sdksyms.sh match the
previous output, otherwise, the code is formatted badly enough that
sdksyms.sh generates incorrect output.
The generated code was compared with the previous version and found to
be essentially identical -- "assert" line numbers and BUILD_TIME were
the only differences found.
The comparison was done with this script:
dir1=$1
dir2=$2
for dir in $dir1 $dir2; do
(cd $dir && find . -name '*.o' | while read file; do
dir=`dirname $file`
base=`basename $file .o`
dump=$dir/$base.dump
objdump -d $file > $dump
done)
done
find $dir1 -name '*.dump' | while read dump; do
otherdump=`echo $dump | sed "s;$dir1;$dir2;"`
diff -u $dump $otherdump
done
Signed-off-by: Keith Packard <keithp@keithp.com>
Acked-by: Daniel Stone <daniel@fooishbar.org>
Acked-by: Alan Coopersmith <alan.coopersmith@oracle.com>
