Commits on Source (34)
-
Arthur Huillet authored
_XReply isn't reentrant, and it can lead to deadlocks when the default error handler is called: _XDefaultError calls exit(1). It is called indirectly by _XReply when a X protocol error comes in that isn't filtered/handled by an extension or the application. This means that if the application (or one of its loaded shared libraries such as the NVIDIA OpenGL driver) has registered any _fini destructor, _fini will get called while still on the call stack of _XReply. If the destructor interacts with the X server and calls _XReply, it will hit a deadlock, looping on the following in _XReply: ConditionWait(dpy, dpy->xcb->reply_notify); It is legal for an application to make Xlib calls during _fini, and that is useful for an OpenGL driver to avoid resource leaks on the X server side, for example in the dlopen/dlclose case. However, the driver can not readily tell whether its _fini is being called because Xlib called exit, or for another reason (dlclose), so it is hard to cleanly work around this issue in the driver. This change makes it so _XReply effectively becomes a no-op when called after _XDefaultError was called, as though an XIOError had happened. The dpy connection isn't broken at that point, but any call to _XReply is going to hang. This is a bit of a kludge, because the more correct solution would be to make _XReply reentrant, maybe by broadcasting the reply_notify condition before calling the default error handler. However, such a change would carry a grater risk of introducing regressions in Xlib. This change will drop some valid requests on the floor, but this should not matter, as it will only do so in the case where the application is dying: X will clean up after it once exit() is done running. There is the case of XSetCloseDownMode(RETAIN_PERMANENT), but an application using that and wishing to clean up resources in _fini would currently be hitting a deadlock, which is hardly a better situation. Signed-off-by:Aaron Plattner <aplattner@nvidia.com> Reviewed-by:
Jamey Sharp <jamey@minilop.net>
-
Alan Coopersmith authored
The XKB Library spec and the man pages for XkbGetNamedIndicator & XkbSetNamedIndicator included a device_spec argument neither function takes, and do not include the XkbGetNamedDeviceIndicator & XkbSetNamedDeviceIndicator variants that do take it (along with two other arguments). This updates them to match the interfaces the code has provided for decades. This has been reported multiple times, so this fixes: https://bugs.freedesktop.org/show_bug.cgi?id=251 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=729812 Sun Bug 4528016 XkbSetNamedIndicator & XkbGetNamedIndicator man pages are wrong (filed: alan.coopersmith@sun.com 2001-11-15 - now aka Oracle bug 15087506) X.Org Group Defect Id #9418 Signed-off-by:
Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by:
Adam Jackson <ajax@redhat.com>
-
Alan Coopersmith authored
Checking a Bool to see if it's NULL does not work well in C. Also reported in https://bugs.freedesktop.org/show_bug.cgi?id=251 Signed-off-by:
-
Alan Coopersmith authored
Includes fix for Solaris Bug 24564279: "XkbKeyNumGroups.3x11 man page contains some malformed text" caused by extra whitespace after .TE macros Signed-off-by: -
Ryan C. Gordon authored
If the "pad" field isn't set, Valgrind will report it as uninitialized memory accesses when the struct is copied into the Display's send buffer. In practice, this is (probably) harmless, but Valgrind is correct in believing it's a bug. https://bugs.freedesktop.org/attachment.cgi?id=133189 Reviewed-by:
-
walter harms authored
Signed-off-by:walter harms <wharms@bfs.de>
-
walter harms authored
Signed-off-by: -
walter harms authored
Signed-off-by: -
walter harms authored
Signed-off-by: -
walter harms authored
Signed-off-by: -
walter harms authored
simplify code by removing unneeded checks Signed-off-by: -
walter harms authored
remove stray extern Signed-off-by: -
walter harms authored
simplify code Signed-off-by: -
walter harms authored
V2: remove unneeded NULL (reported by eric.engestrom@imgtec.com) fix mem leak in error path Signed-off-by: -
walter harms authored
free all mem on error Signed-off-by: -
walter harms authored
You can save a bit of code. The is no need to check XFree arguments bring free_fontdataOM in line with other free function and check for NULL arg Signed-off-by: -
walter harms authored
mark _XDefaultIOError as no_return. No one comes back from exit() ... Signed-off-by: -
walter harms authored
Fixes: warning: variable 'req' set but not used [-Wunused-but-set-variable] by marking req _X_UNUSED Solution was discussed on xorg-devel ML Peter Hutter, Alan Coopersmith Re: [PATCH libX11 3/5] fix: warning: pointer targets in passing argument 2 of '_XSend' differ in signedness [-Wpointer-sign] Signed-off-by: -
wharms authored
-
wharms authored
-
wharms authored
-
Alan Coopersmith authored
Reported by gcc 7.3: GetImage.c:110:25: warning: potential null pointer dereference [-Wnull-dereference] if (planes < 1 || image->height < 1 || image->bytes_per_line < 1 || ~~~~~^~~~~~~~ Introduced by 8ea762f9 in Xlib 1.6.4 Signed-off-by:Emil Velikov <emil.velikov@collabora.com>
-
The _XimCacheStruct structure is followed in memory by two strings containing fname and encoding. The memory was accessed using the last member of the structure `char fname[1]`. That is a lie, prohibits us from using sizeof and confuses checkers. Lets declare it properly as a flexible array, so compilers don't complain about writing past that array. As bonus we can replace the XOffsetOf with regular sizeof. Fixes GCC8 error: In function 'strcpy', inlined from '_XimWriteCachedDefaultTree' at imLcIm.c:479:5, inlined from '_XimCreateDefaultTree' at imLcIm.c:616:2, inlined from '_XimLocalOpenIM' at imLcIm.c:700:5: /usr/include/bits/string_fortified.h:90:10: error: '__builtin_strcpy' forming offset 2 is out of the bounds [0, 1] [-Werror=array-bounds] return __builtin___strcpy_chk (__dest, __src, __bos (__dest)); Caused by this line seemingly writing past the fname[1] array: imLcIm.c:479: strcpy (m->fname+strlen(name)+1, encoding); Reviewed-by:Keith Packard <keithp@keithp.com> Signed-off-by:
Peter Hutterer <peter.hutterer@who-t.net>
-
Bhavi Dhingra authored
https://bugs.freedesktop.org/show_bug.cgi?id=96814 Reviewed-by:
-
Alan Coopersmith authored
These variables store values returned from strlen() as a size_t and are passed to Xmalloc, which expects a size_t, so lets stop converting back and forth to int along the way. Reported by: Konstantin SKliarov Signed-off-by:
Matthieu Herrb <matthieu@herrb.eu>
-
Alan Coopersmith authored
Needs to match one of the regexps shown under https://gcc.gnu.org/onlinedocs/gcc-7.3.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough Signed-off-by:
-
Martin Natano authored
ks_tables.h is always considered out of date due to the forced rebuild of the makekeys util. This means the file is also rebuilt during 'make install', which is usually performed as root, which can to lead permission problems later on. Signed-off-by:
Martin Natano <natano@natano.net> Signed-off-by:
-
Samuel Thibault authored
XkbOpenDisplay returns a pointer to Display, not a Display. Signed-off-by:Samuel Thibault <samuel.thibault@ens-lyon.org>
-
Tobias Stöckmann authored
If a server sends an incorrect length in its response, a client is prone to perform an out of boundary read while processing the data. The length field of xHostEntry is used to specify the amount of bytes used to represent the address. It is 16 bit, which means that it is not possible to perform an arbitrary memory access, but it might be enough to read sensitive information, e.g. malloc-related pointers and offsets. Signed-off-by:
Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by:
-
Tobias Stöckmann authored
The functions XGetFontPath, XListExtensions, and XListFonts are vulnerable to an off-by-one override on malicious server responses. The server replies consist of chunks consisting of a length byte followed by actual string, which is not NUL-terminated. While parsing the response, the length byte is overridden with '\0', thus the memory area can be used as storage of C strings later on. To be able to NUL-terminate the last string, the buffer is reserved with an additional byte of space. For a boundary check, the variable chend (end of ch) was introduced, pointing at the end of the buffer which ch initially points to. Unfortunately there is a difference in handling "the end of ch". While chend points at the first byte that must not be written to, the for-loop uses chend as the last byte that can be written to. Therefore, an off-by-one can occur. I have refactored the code so chend actually points to the last byte that can be written to without an out of boundary access. As it is not possible to achieve "ch + length < chend" and "ch + length + 1 > chend" with the corrected chend meaning, I removed the inner if-check. Signed-off-by: -
Tobias Stöckmann authored
The length value is interpreted as signed char on many systems (depending on default signedness of char), which can lead to an out of boundary write up to 128 bytes in front of the allocated storage, but limited to NUL byte(s). Casting the length value to unsigned char fixes the problem and allows string values with up to 255 characters. Signed-off-by: -
Tobias Stöckmann authored
If the server sends a reply in which even the first string would overflow the transmitted bytes, list[0] (or flist[0]) will be set to NULL and a count of 0 is returned. If the resulting list is freed with XFreeExtensionList or XFreeFontPath later on, the first Xfree call: Xfree (list[0]-1) turns into Xfree (NULL-1) which will most likely trigger a segmentation fault. I have modified the code to return NULL if the first string would overflow, thus protecting the freeing functions later on. Signed-off-by: -
Matthieu Herrb authored
Signed-off-by: -
Matthieu Herrb authored
Signed-off-by:
man/xkb/XkbGetNamedDeviceIndicator.man
0 → 100644
man/xkb/XkbSetNamedDeviceIndicator.man
0 → 100644