If a device property is going to be updated, but failing due
the new value being too big, the buffer isn't freed.
Also compacting the logic for this into small inline function.
Fixes: 948630fa42
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
A client might send a request causing an integer overflow when computing
the total size to allocate in RRChangeProviderProperty().
To avoid the issue, check that total length in bytes won't exceed the
maximum integer value.
CVE-2025-49180
This issue was discovered by Nils Emmerich <nemmerich@ernw.de> and
reported by Julian Suleder via ERNW Vulnerability Disclosure.
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/2024>
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>
Moving payload buffer assembly right into the same branch where the buffer is
allocated, so making the whole code flow easier to understand. Also moving the
byteswap there (when the fields should still be in CPU cache), instead of having
some callback doing it much later, so even more simplication.
As a nice by-product, that's also reducing some analyzer noise.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The code can be much simpler by just using CopySwap32Write().
And we also don't need the callback in WriteSwappedDataToClient(),
just call the corresponding write function directly.
This also makes some analyzer warnings go away.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Simplifying the code flow allocating/checking/copying some buffers in
RRConfigureOutputProperty() and RRConfigureProviderProperty() so it's
easier to understand for both the human reader as well as the analyzer.
Depending on whether we have elements to process, a temporary buffer needs
to be allocated, checked for successful allocation and copy over data. The
way it's currently done is technically correct, but unnecessarily complex to
understand: instead of just branching on whether there are elements and doing
all the buffer-related things only then, the branching is done just somewhere
in the middle, only on checking for allocation failure, and relying on both
calloc() and memcpy() not doing weird things when size is zero.
It's easy to simplify by putting it all behind one if statement and so make
things easier for both human reader as well as the analyzer (so it's not
spilling out false alarms here anymore) and also drops unnecessary calls
in the zero-size case.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Those aren't used by any drivers and never should so, thus no need to
keep them exported.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Simplify reply payload preparation and sendout by using SwapShort()
and SwapLong() instead of WriteToClientSwapped() and callbacks.
This also allows even further simplifications by using generic macros
for the request send path.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
We can rely on everything being cleared. And usually even faster, as the
compiler can emit optimized instructions for clearing a whole block at once.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
Improve readability, move the declarations to where they're needed first
and get rid of extra individual assignments. In some cases this should also
allow the compiler to produce a bit more efficient code.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1794>
The authorative source of the request frame size is client->req_len,
especially with big requests larger than 2^18 bytes.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1639>
Affected are ProcRRChangeProviderProperty and ProcRRChangeOutputProperty.
See also xserver@8f454b79 where this same bug was fixed for the core
protocol and XI.
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->nUnits 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->nUnits bytes, i.e. 4GB.
CVE-2023-6478, ZDI-CAN-22561
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
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>
Reported by parfait 1.0:
Error: Memory leak (CWE 401)
Memory leak of pointer 'prop' allocated with RRCreateProviderProperty(property)
at line 221 of randr/rrproviderproperty.c in function 'RRChangeProviderProperty'.
'prop' allocated at line 155 with RRCreateProviderProperty(property).
prop leaks when pending != 0 at line 161.
Error: Memory leak (CWE 401)
Memory leak of pointer 'prop' allocated with RRCreateProviderProperty(property)
at line 345 of randr/rrproviderproperty.c in function 'RRConfigureProviderProperty'.
'prop' allocated at line 333 with RRCreateProviderProperty(property).
at line 349 of randr/rrproviderproperty.c in function 'RRConfigureProviderProperty'.
'prop' allocated at line 333 with RRCreateProviderProperty(property).
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
RandR 1.4 was going through review in parallel with main batch of
C99 initialization changes - sync up now that both have landed.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
RandR 1.4 was going through review in parallel with main batch of
C99 initialization changes - sync up now that both have landed.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
This adds the initial provider object and provider property
support to the randr dix code.
v2: destroy provider in screen close
v2.1: fix whitespace
v3: update for latest rev of protocol + renumber after 1.4 tearout.
v4: fix logic issue, thanks Samsagax on irc
v5: keithp's review: fix current_role, fix copyrights, fix master
reporting crtc/outputs.
v6: port to new randr interface, drop all set role bits for now
v7: drop devPrivate in provider, not needed, add BadMatch returns
for NULL SetProviderOffloadSink and SetProviderOutputSource, drop
the old typedef.
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Enrico Weigelt, metux IT consult
Olivier Fourdan
Peter Hutterer
Alan Coopersmith
Keith Packard
Dave Airlie