_XkbCheckRequestBounds assumes that from..to is at least one byte.
However, request strings can be empty, causing spurious failures in
XkbGetKbdByName calls. To avoid this, before checking bounds make
sure that the length is nonzero.
GetCountedString did a check for the whole string to be within the
request buffer but not for the initial 2 bytes that contain the length
field. A swapped client could send a malformed request to trigger a
swaps() on those bytes, writing into random memory.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Each string length field was accessed before checking whether that byte
was actually part of the client request. No real harm here since it
would immediately fail with BadLength anyway, but let's be correct here.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This request accessed &stuff[1] before length-checking everything. The
check was performed afterwards so invalid requests would return
BadLength anyway, but let's do this before we actually access the
memory.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
No validation of the various fields on that report were done, so a
malicious client could send a short request that claims it had N
sections, or rows, or keys, and the server would process the request for
N sections, running out of bounds of the actual request data.
Fix this by adding size checks to ensure our data is valid.
ZDI-CAN 16062, CVE-2022-2319.
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>
XKB often uses a FooCheck and Foo function pair, the former is supposed
to check all values in the request and error out on BadLength,
BadValue, etc. The latter is then called once we're confident the values
are good (they may still fail on an individual device, but that's a
different topic).
In the case of XkbSetDeviceInfo, those functions were incorrectly
named, with XkbSetDeviceInfo ending up as the checker function and
XkbSetDeviceInfoCheck as the setter function. As a result, the setter
function was called before the checker function, accessing request
data and modifying device state before we ensured that the data is
valid.
In particular, the setter function relied on values being already
byte-swapped. This in turn could lead to potential OOB memory access.
Fix this by correctly naming the functions and moving the length checks
over to the checker function. These were added in 87c64fc5b0 to the
wrong function, probably due to the incorrect naming.
Fixes ZDI-CAN 16070, CVE-2022-2320.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Introduced in c06e27b2f6
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Most similar loops here use a pointer that advances with each loop
iteration, let's do the same here for consistency.
No functional changes.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Sick of fighting vim and git from trying to add this fix with every
commit iteration...
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
This header merely defines the various protocol request handlers, so
let's rename it to something less generic and remove its include from
all the files that don't actually need it (which is almost all of them).
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
As the comment says:
"symsPerKey/mapWidths must be filled regardless of client-side flags"
so we always have to call CheckKeyTypes which will notably fill mapWidths
and nTypes. That is needed for CheckKeySyms to work since it checks the
width. Without it, any request with XkbKeySymsMask but not
XkbKeyTypesMask will fail because of the missing width information, for
instance this:
XkbDescPtr xkb;
if (!(xkb = XkbGetMap (dpy, XkbKeyTypesMask|XkbKeySymsMask, XkbUseCoreKbd))) {
fprintf (stderr, "ERROR getting map\n");
exit(1);
}
XFlush (dpy);
XSync (dpy, False);
XkbMapChangesRec changes = { .changed = 0 };
int oneGroupType[XkbNumKbdGroups] = { XkbOneLevelIndex };
if (XkbChangeTypesOfKey(xkb, keycode, 1, XkbGroup1Mask, oneGroupType, &changes)) {
fprintf(stderr, "ERROR changing type of key\n");
exit(1);
}
XkbKeySymEntry(xkb,keycode,0,0) = keysym;
if (!XkbChangeMap(dpy,xkb,&changes)) {
fprintf(stderr, "ERROR changing map\n");
exit(1);
}
XkbFreeKeyboard (xkb, 0, TRUE);
XFlush (dpy);
XSync (dpy, False);
This had being going under the radar since about ever until commit
de940e06f8 ("xkb: fix key type index check
in _XkbSetMapChecks") fixed checking the values of kt_index, which was
previously erroneously ignoring errors and ignoring all other checks, just
because nTypes was not set, precisely because CheckKeyTypes was not called.
Note: yes, CheckKeyTypes is meant to be callable without XkbKeyTypesMask, it
does properly check for that and just fills nTypes and mapWidths in that
case.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
The previous if/else condition resulted in us always setting the key
type count to the current number of key types. Split this up correctly.
Regression introduced in de940e06f8Fixes#1249
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Commit 446ff2d317 added checks to
prevalidate the size of incoming SetMap requests.
That commit checks for the XkbSetMapResizeTypes flag to be set before
allowing key types data to be processed.
key types data can be changed or even just sent wholesale unchanged
without the number of key types changing, however. The check for
XkbSetMapResizeTypes rejects those legitimate requests. In particular,
XkbChangeMap never sets XkbSetMapResizeTypes and so always fails now
any time XkbKeyTypesMask is in the changed mask.
This commit drops the check for XkbSetMapResizeTypes in flags when
prevalidating the request length.
xkb.c: In function ‘ProcXkbSetMap’:
xkb.c:2747:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
2747 | DeviceIntPtr master = GetMaster(dev, MASTER_KEYBOARD);
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Avoid out of bounds memory accesses on too short request.
ZDI-CAN 11572 / CVE-2020-14360
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
ZDI-CAN 11389 / CVE-2020-25712
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
CVE-2020-14345 / ZDI 11428
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
This code block was moved from a function that returns 0 for failure to a
function that returns 0 for Success in commit
649293f6b6. Change the return value to
BadValue to match the other checks in _XkbSetMapChecks.
Set nTypes to xkb->map->num_types when XkbKeyTypesMask is not set, to
allow requests with the XkbKeyTypesMask flag unset in stuff->present to
succeed.
Fixes a potential heap smash when client->swapped is true, because the
remainder of the request will not be swapped after "return 0", but
_XkbSetMap will be called anyway (because 0 is Success).
Signed-off-by: Peter Harris <pharris@opentext.com>
The server swaps part of the request in _XkbSetMapChecks instead of
SProcXkbSetMap (presumably because walking the XkbSetMap request is hard,
and we don't want to maintain another copy of that code).
Swap the first time _XkbSetMapChecks is called, not the second time.
Signed-off-by: Peter Harris <pharris@opentext.com>
XKB stores some stuff in the ClientRec that, style-wise, should probably
be in a client private. vMinor tracks the client's idea of the XKB
minor version, but is never read, we can just nuke it. vMajor is only
used for a bug-compat workaround for X11R6.0-vintage clients. We're
only using though (1<<4) for xkbClientFlags in the protocol, so we can
pack that field down to a u8 and store the bug-compat flag there.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Regression introduce by ac164e5887 which calls
XkbUpdateAllDeviceIndicators() with two NULL arguments. A few layers down into
the stack and we triggered a NULL-pointer dereference. In theory a NULL cause
is acceptable since we don't actually change modifier state here. Instead of
updating all places to check for NULL just set the cause to the client
request and go to the pub.
https://bugs.freedesktop.org/show_bug.cgi?id=96384
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: Daniel Stone <daniels@collabora.com>
When NumLock is on and a new keymap is applied, the next modifier state
change will turn off that LED (but leave the state enabled). The cause
for this is a bit convoluted:
* the SLI explicitState is copied from the current state in
ProcXkbGetKbdByName. Thus, if NumLock is on, that state is 0x2.
* on the next modifier key press (e.g. Shift), XkbApplyState() calls into
XkbUpdateIndicators() -> XkbUpdateLedAutoState() to update SLIs (if any)
for the currently changed modifier. But it does so with a mask only for
the changed modifier (i.e. for Shift).
* XkbUpdateLedAutoState() calculates the state based on this mask and
ends up with 0 because we don't have a Shift LED and we masked out the
others.
* XkbUpdateLedAutoState() compares that state with the previous state
(which is still 0x2) and then proceeds to turn the LED off
This doesn't happen in the normal case because either the mask
encompasses all modifiers or the state matches of the masked-out
modifiers matches the old state.
Avoid this issue by forcing an SLI update after changing the keymap.
This updates the sli->effectiveState and thus restores everything to
happy working order.
https://bugzilla.redhat.com/show_bug.cgi?id=1047151
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Ensure that the given strings length in an XkbSetGeometry request remain
within the limits of the size of the request.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The XkbSetGeometry request embeds data which needs to be swapped when the
server and the client have different endianess.
_XkbSetGeometry() invokes functions that swap these data directly in the
input buffer.
However, ProcXkbSetGeometry() may call _XkbSetGeometry() more than once
(if there is more than one keyboard), thus causing on swapped clients the
same data to be swapped twice in memory, further causing a server crash
because the strings lengths on the second time are way off bounds.
To allow _XkbSetGeometry() to run reliably more than once with swapped
clients, do not swap the data in the buffer, use variables instead.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
When _XkbCheckAtoms returns NULL for an error, it always sets the
error return code, but GCC can't figure that out, so just initialize
the local variable, 'bad', in _XkbSetNamesCheck to eliminate the warning.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
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>
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>
Xlib doesn't use this value (it computes it from the reply length
instead) which is why nobody has noticed yet. But the spec
http://www.x.org/releases/X11R7.7/doc/kbproto/xkbproto.html
says that it should be set.
Signed-off-by: Peter Harris <pharris@opentext.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
xkb.c: In function '_XkbSetNamesCheck':
xkb.c:3987:18: warning: variable 'names' set but not used
[-Wunused-but-set-variable]
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
The formatter confused address operators preceded by casts with
bitwise-and expressions, placing spaces on either side of both.
That syntax isn't used by ordinary address operators, however,
so fix them for consistency.
Signed-off-by: Yaakov Selkowitz <yselkowitz@users.sourceforge.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
No-one uses this - not xkbcomp, not GNOME, not KDE. The preferred way
to deal with component listing (which gives you RMLVO rather than
KcCGST) is to use the XML files on the client side.
Indeed, a couple of hours after making this commit, it emerged that all
*.dir files built with xkbcomp 1.1.1 (released two years ago) and later
have been catastrophically broken and nearly empty. So I think that's
reasonable proof that no-one uses them.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Create extinit.h (and xf86Extensions.h, for Xorg-specific extensions) to
hold all our extension initialisation prototypes, rather than
duplicating them everywhere.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Tested-by: Daniel Stone <daniel@fooishbar.org>
Casting return to (void) was used to tell lint that you intended
to ignore the return value, so it didn't warn you about it.
Casting the third argument to (char *) was used as the most generic
pointer type in the days before compilers supported C89 (void *)
(except for a couple places it's used for byte-sized pointer math).
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Tested-by: Daniel Stone <daniel@fooishbar.org>
If the typedef wasn't perfect, indent would get confused and change:
foo = (SomePointlessTypedef *) &stuff[1];
to:
foo = (SomePointlessTypedef *) & stuff[1];
Fix this up with a really naïve sed script, plus some hand-editing to
change some false positives in XKB back.
Signed-off-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>
Caught during review of e095369bf.
Signed-off-by: Peter Harris <pharris@opentext.com>
Reviewed-by-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Matt Turner <mattst88@gmail.com>
Swapping the wrong size was never caught because swap{l,s} are macros.
It's clear in the case of Xext/xres.c, that the author believed
client_major/minor to be CARD16 from looking at the code in the first
hunk.
v2: dmx.c fixes from Keith.
Reviewed-by: Peter Harris <pharris@opentext.com>
Signed-off-by: Matt Turner <mattst88@gmail.com>
Also, fix whitespace, mainly around
swaps(&rep.sequenceNumber)
Reviewed-by: Peter Harris <pharris@opentext.com>
Signed-off-by: Matt Turner <mattst88@gmail.com>
Bugfix for broken xkbcomp: if we encounter an XFree86Private action with
Any+AnyOfOrNone(All), then we skip the interp as broken. Versions of
xkbcomp below 1.2.2 had a bug where they would interpret a symbol that
couldn't be found in an interpret as Any. So, an
XF86LogWindowTree+AnyOfOrNone(All) interp that triggered the PrWins
action would make every key without an action trigger PrWins if libX11
didn't yet know about the XF86LogWindowTree keysym. None too useful.
We only do this for XFree86 actions, as the current XKB dataset relies
on Any+AnyOfOrNone(All) -> SetMods for Ctrl in particular.
See xkbcomp commits 2a473b906943ffd807ad81960c47530ee7ae9a60 and
3caab5aa37decb7b5dc1642a0452efc3e1f5100e for more details.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
When we change the keymap on a device, send the NewKeyboardNotify for
that device before we copy the keymap to and notify for its attached
master/slave devices.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Previously we had:
foreach (device + slaves of device) {
XkbCopyDeviceKeymap(i, device);
[...]
}
if (device was last slave of its MD) {
XkbCopyDeviceKeymap(master, device);
}
and now:
foreach (device + slaves of device + MD if device was last slave) {
XkbCopyDeviceKeymap(i, device);
[...]
}
As an extra bonus, when changing the keymap on a slave device, we now
ensure the LED info on the master is kept in sync.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Replace:
for (stuff; things; etc) {
if (misc || other) {
[...]
}
}
with:
for (stuff; things; etc) {
if (!misc && !other)
continue;
[...]
}
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
In the XKB GetKeyboardByName handler, we had the following pseudocode:
if (device was last slave of its MD) {
XkbCopyDeviceKeymap(master, slave);
XkbSendNewKeyboardNotify(slave, ¬ify);
}
Even if the SendNewKeyboardNotify line nominated the correct device,
which it didn't, it's unnecessary as XkbCopyDeviceKeymap already sends a
NewKeyboardNotify on the destination device.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Valgrind complains about uninitialized data being written to clients.
Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi>
Signed-off-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
The removal of the double-use will cause some suble bugs as some conditions
to check for the dev->u.master case were broken and also evaluated as true
if lastSlave was set (instead of master).
Also breaks the input ABI.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Benjamin Tissoires <tissoire@cena.fr>
And copy into the master keyboard, not just the directly attached device.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Benjamin Tissoires <tissoire@cena.fr>
This is not a straightforward search/replacement due to a long-standing
issue.
dev->u.master is the same field as dev->u.lastSlave. Thus, if dev is a master
device, a check for dev->u.master may give us false positives and false
negatives.
The switch to IsFloating() spells out these cases and modifies the
conditions accordingly to cover both cases.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Benjamin Tissoires <tissoire@cena.fr>
The two functions have identical semantics, including safely returning
NULL when NULL is passed in (which POSIX strdup does not guarantee).
Some callers could probably be adjusted to call libc strdup directly,
when we know the input is non-NULL.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
==9999== Syscall param writev(vector[...]) points to uninitialised byte(s)
==9999== at 0x4AB5154: writev (writev.c:51)
==9999== by 0x7C7C3: _XSERVTransWritev (Xtrans.c:912)
==9999== by 0x61C8B: FlushClient (io.c:924)
==9999== by 0x62423: WriteToClient (io.c:846)
==9999== by 0xCE39B: XkbSendMap (xkb.c:1408)
==9999== by 0xD247B: ProcXkbGetKbdByName (xkb.c:5814)
==9999== by 0x4AB53: Dispatch (dispatch.c:432)
==9999== by 0x205BF: main (main.c:291)
==9999== Address 0x557eb68 is 40 bytes inside a block of size 4,096 alloc'd
==9999== at 0x48334A4: calloc (vg_replace_malloc.c:467)
==9999== by 0x62567: WriteToClient (io.c:1065)
==9999== by 0x452EB: ProcEstablishConnection (dispatch.c:3685)
==9999== by 0x4AB53: Dispatch (dispatch.c:432)
==9999== by 0x205BF: main (main.c:291)
==9999== Uninitialised value was created by a stack allocation
==9999== at 0xD1910: ProcXkbGetKbdByName (xkb.c:5559)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Oliver McFadden <oliver.mcfadden@nokia.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
==537== Syscall param writev(vector[...]) points to uninitialised byte(s)
==537== at 0x4AB7154: writev (writev.c:51)
==537== by 0x8935B: _XSERVTransWritev (Xtrans.c:912)
==537== by 0x6C55F: FlushClient (io.c:924)
==537== by 0x6CCF3: WriteToClient (io.c:846)
==537== by 0xD51D3: XkbSendNames (xkb.c:3765)
==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825)
==537== by 0x27B7B: Dispatch (dispatch.c:432)
==537== by 0x205B7: main (main.c:291)
==537== Address 0x55899f2 is 154 bytes inside a block of size 1,896 alloc'd
==537== at 0x4834C48: malloc (vg_replace_malloc.c:236)
==537== by 0xD47AF: XkbSendNames (xkb.c:3642)
==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825)
==537== by 0x27B7B: Dispatch (dispatch.c:432)
==537== by 0x205B7: main (main.c:291)
==537== Uninitialised value was created by a heap allocation
==537== at 0x4834C48: malloc (vg_replace_malloc.c:236)
==537== by 0xD47AF: XkbSendNames (xkb.c:3642)
==537== by 0xD8183: ProcXkbGetKbdByName (xkb.c:5825)
==537== by 0x27B7B: Dispatch (dispatch.c:432)
==537== by 0x205B7: main (main.c:291)
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Oliver McFadden <oliver.mcfadden@nokia.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com>
When XkbChangeEnabledControls is called to disable key repetition of a
certain key (or keys), currently ongoing repetition of that key was
not cancelled. It was cancelled if ChangeKeyboardControl was used to
disable key repetition globally.
Reviewed-by: Rami Ylimäki <rami.ylimaki@vincit.fi>
Reviewed-by: Dirk Wallenstein <halsmit@t-online.de>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Even if a client does not modify the symbols, symsPerKey and mapWidths must
be filled from the current configuration. Both arrays are then passed into
other functions (pending the right flag), thus they must contain valid
values regardless of the XkbKeySymsMask flag in req->present.
X.Org Bug 30527 <http://bugs.freedesktop.org/show_bug.cgi?id=30527>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Helps debugging greatly, random 8 or 16 bit values can sometimes look like
valid values, causing much excitement on the client front.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
map is allocated but not freed if reply length and data don't match.
Signed-off-by: Pauli Nieminen <ext-pauli.nieminen@nokia.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Devices that are both pointers and keyboards are not affected by keyboard
changes as their master device is a master pointer, not a master keyboard.
Use GetMaster() instead to ensure devices that are attached to the paired
master pointer device will still be update.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Make sure all of the private keys used by the test code are
initialized before being used.
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Robert Hooker <sarvatt@ubuntu.com>
Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
Reviewed-by: Marcin Baczyński <marbacz@gmail.com>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
This patch only changes the API, not the implementation of the
devPrivates infrastructure. This will permit a new devPrivates
implementation to be layed into the server without requiring
simultaneous changes in every devPrivates user.
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Tiago Vignatti <tiago.vignatti@nokia.com>
Just let Dispatch() check for a noClientException, rather than making
every single dispatch procedure take care of it.
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
The only remaining X-functions used in server are XNF*, the rest is converted to
plain alloc/calloc/realloc/free/strdup.
X* functions are still exported from server and x* macros are still defined in
header file, so both ABI and API are not affected by this change.
Signed-off-by: Mikhail Gusarov <dottedmag@dottedmag.net>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dan Nicholson <dbn.lists@gmail.com>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
Please no extension-specific macros for memory allocation.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Dan Nicholson <dbn.lists@gmail.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Convert all calls of CreateNewResourceType to pass name argument
Breaks DIX ABI.
ABI versions bumped:
Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Calls RegisterResourceName to record the type name for
use by X-Resource, XACE/SELinux/XTsol, and DTrace.
Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Make sure to check return value before setting bitmask flags.
For most calls, just fails to init the extension. Since Xinput
already calls FatalError() on initialization failure, so does
failure to allocate Xinput's resource type.
Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
As the comment for the function states, led_return is undefined if map is
NULL. We might as well skip writing to it then.
Found by clang.
Reported-by: Tomas Carnecky <tom@dbservice.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
This avoids NULL from being passed to memcpy() later in the code. While
that wasn't an issue before - that value being NULL implied 'size == 0'
so memcpy() wouldn't try to dereference it - it made the code harder
to read and also confused clang.
Signed-off-by: Tomas Carnecky <tom@dbservice.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This request is used to get the current keyboard group and is called from
GTK. It does not return an actual keymap (aside from modifiers) so it
should be safe to relax the permission on it. However it does return
button state information which should be controlled through a separate
pointer Read check.
Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
include/protocol-versions.h specifies each extension version as supported by
the server and sent back on the wire to the client.
This fixes up several issues with the server potentially reporting a higher
version of the protocol if recompiled against a newer version of the
protocol.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Rémi Cardona <remi@gentoo.org>
Acked-by: Julien Cristau <jcristau@debian.org>
If the layout is changed on a master's lastSlave, the master needs to change
layout immediately. Otherwise, the master stays on the same layout until the
lastSlave changes - which may not happen if only a single keyboard is
available.
X.Org Bug 21859 <http://bugs.freedesktop.org/show_bug.cgi?id=21859>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
isMaster is not enough as long as we differ between master pointers and
keyboard. With flexible device classes, the usual checks for whether a
master device is a pointer (currently check for ->button, ->valuators or
->key) do not work as an SD may post an event through a master and mess this
check up.
Example, a device with valuators but no buttons would remove the button
class from the VCP and thus result in the
IsPointerDevice(inputInfo.pointer) == FALSE.
This will become worse in the future when new device classes are introduced
that aren't provided in the current system (e.g. a switch class).
This patch replaces isMaster with "type", one of SLAVE, MASTER_POINTER and
MASTER_KEYBOARD. All checks for dev->isMaster are replaced with an
IsMaster(dev).
In ProcXkbGetKbdByName, mrep.firstVModMapKey, .nVModMapKeys and
.totalVModMapKeys were not initialized, contained random values and caused
accesses to unallocated and later modified memory, causing
XkbSizeVirtualModMap and XkbWriteVirtualModMap to see different number of
nonzero values, resulting in writes past the end of an array in XkbSendMap.
This patch initializes those values sensibly and reverts commits 5c0a2088 and
6dd4fc46, which have been plain non-sense.
Signed-off-by: Tomas Janousek <tomi@nomi.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This type is only used in XI to give a hint of what type this device may be.
Call it xinput_type for clarity.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
In ProcXkbGetKbdByName, mrep.firstVModMapKey, .nVModMapKeys and
.totalVModMapKeys were not initialized, contained random values and caused
accesses to unallocated and later modified memory, causing
XkbSizeVirtualModMap and XkbWriteVirtualModMap to see different number of
nonzero values, resulting in writes past the end of an array in XkbSendMap.
This patch initializes those values sensibly and reverts commits 5c0a2088 and
6dd4fc46, which have been plain non-sense.
Signed-off-by: Tomas Janousek <tomi@nomi.cz>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
When ProcXkbSetNamedIndicator is called on a core device, and we
walk the slaves to set the LED's on each of them, ignore any slaves
that do not have either a KbdFeedbackCtrl or LedCtrl structure.
(This is much more critical in xserver-1.5-branch, where we walk
*all* devices, not just the slaves of the specified master, and
thus return failure when setting an LED on the Core Keyboard and
hit a xf86-input-mouse device with no LED's to set.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Changes MakeAtom to take a const char * and NameForAtom to return them,
since many callers pass pointers to constant strings stored in read-only
ELF sections. Updates in-tree callers as necessary to clear const
mismatch warnings introduced by this change.
Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Instead of always keeping two copies of the keymap, only generate the
core keymap from the XKB keymap when we really need to, and use the XKB
keymap as the canonical keymap.
Signed-off-by: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>