This function is a funny beast: it assembles and writes out an xkbGetMapReply,
called in two different cases, ProcXkbGetMap() as well as ProcXkbGetKbdByName().
In the latter case the whole reply is contained in another one. That's the reason
why it's payload size is computed separately - the caller must know that in order
to set up the container's reply size correctly.
As preparation for upcoming simplifications of the reply send path, splitting off
this function into pieces: XkbAssembleMap() just assembles the reply payload,
while it's callers now responsible for preparing the request header and writing
out both pieces.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
For easier reading, move th sub-reply structs down to where they're used
first and use static initialization for the common fields.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Move down the declaration of the reply struct, right before swapping and sending
and use static initialization.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The function doesn't need to pass anything back via this pointer, it's
the last consumer of this struct. Make understanding the code a bit easier
and clear the road for further simplifications by passing the struct as
value instead of pointer.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The function doesn't need to pass anything back via this pointer, it's
the last consumer of this struct. Make understanding the code a bit easier
and clear the road for further simplifications by passing the struct as
value instead of pointer.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make the code flow a bit easier to understand and allow further simplification
by now just having to write out one additional payload as one block.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Putting both payload pieces into one buffer, so it can be written out
with only one call.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's hard to see which fields of the xkbGetDeviceInfoReply struct it's
reading or writing, and that complicates further simplifications of the
caller. So instead let the caller pass in the relevant fields and do the
modifications on the reply structs on its own.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
A bit simplification in code flow.
The extra length check (did we write as much as intended?) isn't necessary,
since the buffer size is computed by the very same data before this
function is called.
Hint: the size computation must be done before calling this one, because
the reply might be encapsulated in another one (xkbGetKbdByName).
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's not passing back any data via that pointer and actually the last
consumer of it. Changing it to value instead of pointer clears the
road for further simplifications by subsequent patches.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make it a bit simpler and easier to read.
calloc() and WriteToClient() can handle zero lengths very well.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's not passing back any data via that pointer and actually the last
consumer of it. Changing it to value instead of pointer clears the
road for further simplifications by subsequent patches.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's not passing back any data via that pointer and actually the last
consumer of it. Changing it to value instead of pointer clears the
road for further simplifications by subsequent patches.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
We don't need the whole struct here, especially do we not wanna change it.
Therefore only pass in what's really needed, so it gets easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Use static initializaton as much as possible and drop unnecessary
or duplicate zero assignments.
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>
The freeRules parameter is always set to TRUE, meaning always free the
XkbRF_RulesRec struct. Therefore also no need to clear out fields that
aren't going to be reused again, ever.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1840>
Passing a negative value in `needed` to the `XkbResizeKeyActions()`
function can create a `newActs` array of an unespected size.
Check the value and return if it is invalid.
This error has been found by a static analysis tool. This is the report:
Error: OVERRUN (CWE-119):
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: cond_const:
Checking "xkb->server->size_acts == 0" implies that
"xkb->server->size_acts" is 0 on the true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: buffer_alloc:
"calloc" allocates 8 bytes dictated by parameters
"(size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts)"
and "8UL".
libX11-1.8.7/src/xkb/XKBMAlloc.c:811: var_assign:
Assigning: "newActs" = "calloc((size_t)((xkb->server->size_acts == 0) ? 1 : xkb->server->size_acts), 8UL)".
libX11-1.8.7/src/xkb/XKBMAlloc.c:815: assignment:
Assigning: "nActs" = "1".
libX11-1.8.7/src/xkb/XKBMAlloc.c:829: cond_at_least:
Checking "nCopy > 0" implies that "nCopy" is at least 1 on the
true branch.
libX11-1.8.7/src/xkb/XKBMAlloc.c:830: overrun-buffer-arg:
Overrunning buffer pointed to by "&newActs[nActs]" of 8 bytes by
passing it to a function which accesses it at byte offset 15
using argument "nCopy * 8UL" (which evaluates to 8).
# 828|
# 829| if (nCopy > 0)
# 830|-> memcpy(&newActs[nActs], XkbKeyActionsPtr(xkb, i),
# 831| nCopy * sizeof(XkbAction));
# 832| if (nCopy < nKeyActs)
(cherry picked from xorg/lib/libx11@af1312d2873d2ce49b18708a5029895aed477392)
Signed-off-by: José Expósito <jexposit@redhat.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1821>
If XkbChangeTypesOfKey() is called with nGroups == 0, it will resize the
key syms to 0 but leave the key actions unchanged.
If later, the same function is called with a non-zero value for nGroups,
this will cause a buffer overflow because the key actions are of the wrong
size.
To avoid the issue, make sure to resize both the key syms and key actions
when nGroups is 0.
CVE-2025-26597, ZDI-CAN-25683
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>
The computation of the length in XkbSizeKeySyms() differs from what is
actually written in XkbWriteKeySyms(), leading to a heap overflow.
Fix the calculation in XkbSizeKeySyms() to match what kbWriteKeySyms()
does.
CVE-2025-26596, ZDI-CAN-25543
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>
The code in XkbVModMaskText() allocates a fixed sized buffer on the
stack and copies the virtual mod name.
There's actually two issues in the code that can lead to a buffer
overflow.
First, the bound check mixes pointers and integers using misplaced
parenthesis, defeating the bound check.
But even though, if the check fails, the data is still copied, so the
stack overflow will occur regardless.
Change the logic to skip the copy entirely if the bound check fails.
CVE-2025-26595, ZDI-CAN-25545
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>
