When _XGetRequest() detects that requested length exceeds remaining display
output buffer capacity it would return NULL. GetResReq() macro obtains "req"
pointer from a call to _XGetRequest() and then proceeds to assign request id
through "req" pointer which leads to NULL pointer dereference in this case.
Fix this by checking if "req" is valid before assigning request id.
Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
This patch is based on research done by Dmitry Osipenko to uncover the
cause of a large class of Xlib lockups.
_XError must unlock and re-lock the display around the call to the
user error handler function. When re-locking the display, two
functions are called to ensure that the display is ready to generate a request:
_XIDHandler(dpy);
_XSeqSyncFunction(dpy);
The first ensures that there is at least one XID available to use
(possibly calling _xcb_generate_id to do so). The second makes sure a
reply is received at least every 65535 requests to keep sequence
numbers in sync (possibly generating a GetInputFocus request and
synchronously awaiting the reply).
If the second of these does generate a GetInputFocus request and wait
for the reply, then a pending error will cause recursion into _XError,
which deadlocks the display.
One seemingly easy fix is to have _XError avoid those calls by
invoking InternalLockDisplay instead of LockDisplay. That function
does everything that LockDisplay does *except* call those final two
functions which may end up receiving an error.
However, that doesn't protect the system from applications which call
some legal Xlib function from within their error handler. Any Xlib
function which cannot generate protocol or wait for events is valid,
including many which invoke LockDisplay.
What we need to do is make LockDisplay skip these two function calls
precisely when it is called from within the _XError context for the
same display.
This patch accomplishes this by creating a list of threads in the
display which are in _XError, and then having LockDisplay check the
current thread against those list elements.
Inspired-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
This function complements XSetIOErrorHandler(), allowing to override
the default behavior that trusts on I/O errors never coming back
(i.e. exit()ing the process).
This is meant as a mechanism for Wayland compositors (that are too
a X11 client + compositing manager) to unfasten seatbelts and jump
through the car window. It might get lucky and land on a stack of
pillows.
In consequence, some functions labeled as _X_NORETURN can as a
matter of fact return. So those hints were removed.
Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Allows us to depend on _X_COLD directly instead of having to check for it.
(Since we also use _X_UNUSED, 7.0.22 or later was implicitly required
already but not checked for.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
This causes issues when compiling code for C++17.
While here, make function prototype match the header with regards
to removal of another register keyword.
This patch is based on a suggestion made by Uli Schlachter in a comment
to the bug report https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93.
Explanation of the bug (given by Uli Schlachter as well):
An error was received and handled. Since there was an error callback set,
Xlib unlocks the display, runs the error callback, and then locks the display
again. This goes through _XLockDisplay and then calls _XSeqSyncFunction.
On this "lock the thing"-path, Xlib notices that sequence numbers are close to
wrap-around and tries to send a GetInputFocus request. However, the earlier
calls already registered themselves as "we are handling replies/errors, do
not interfere!" and so the code here waits for "that other thread" to be done
before it continues. Only that there is no other thread, but it is this thread
itself and thus a deadlock follows.
The bug is relatively easy to reproduce on any desktop environment by
using actively a touchscreen input that supports multitouch, i.e. practically
all mobile devices are affected.
Fixes: https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93
Suggested-by: Uli Schlachter <psychon@znc.in>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Reported-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
The basic rule "put parantheses around macro parameters" should be
observed where possible. Otherwise code like
ConnectionNumber(foo = bar);
fails to compile. (It obviously passes if ConnectionNumber is a C
function.) There are several other macros amended for the same reason.
This bug appeared while building http://ioccc.org/1993/cmills.c, so
historically it was not present.
Signed-off-by: Dominik Muth <muth@nxdomain.no-ip.biz>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The only use of XCONN_CHECK_FREQ was removed in commit 15e5eaf628
when we dropped the old Xlib connection handling in favor of xcb's.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Make use of the new 64-bit sequence number API in XCB 1.11.1 to avoid
the 32-bit sequence number wrap in libX11.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71338
Signed-off-by: Christian Linhart <chris@demorecorder.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
C89 or bust! This was documented as being needed for "only Lynx,
Linux-libc5, OS/2" and has never been enabled in modular builds,
since none of those platforms have had anyone step up to add support
since the X11R7 conversion to autotools.
Mostly performed with unifdef -UX_LOCALE, followed by removal of files
left without any purpose, and manual cleanup of remaining references.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Try to offset the cost of all the recent checks we've added by giving
the compiler a hint that the branches that involve us eating data
are less likely to be used than the ones that process it.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Matches the units of the length field in X protocol replies, and provides
a single implementation of overflow checking to avoid having to replicate
those checks in every caller.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
MakeBigReq inserts a length field after the first 4 bytes of the request
(after req->length), pushing everything else back by 4 bytes.
The current memmove moves everything but the first 4 bytes back.
If a request aligns to the end of the buffer pointer when MakeBigReq is
invoked for that request, this runs over the buffer.
Instead, we need to memmove minus the first 4 bytes (which aren't moved),
minus the last 4 bytes (so we still align to the previous tail).
The 4 bytes that fell out are already handled with Data32, which will
handle the buffermax correctly.
The case where req->length = 1 was already not functional.
Reported by Abhishek Arya <inferno@chromium.org>.
https://bugzilla.mozilla.org/show_bug.cgi?id=803762
Reviewed-by: Jeff Muizelaar <jmuizelaar@mozilla.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(mostly performed with unifdef, followed by some manual cleanup of
the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
WORD64 seems to have only been defined in <X11/Xmd.h> when building for
CRAY, to handle int being a 64-bit value (ILP64, not LP64) and having
64-bit alignment requirements.
It hadn't been fully supported even before autotooling, as can be
seen by removed code such as:
#ifdef WORD64
_XkbWriteCopyData32 Not Implemented Yet for sizeof(int)==8
#endif
(mostly performed with unifdef, followed by some manual cleanup of
the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
MUSTCOPY seems to have only been defined in <X11/Xmd.h> when building for
CRAY, to handle missing some sizes of integer type.
(mostly performed with unifdef, followed by some manual cleanup of
spacing/indenting in the remaining code)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Casts were annoying gcc by dropping constness when changing types,
when routines simply either copy data into the request buffer or
send it directly to the X server, and never modify the input.
Fixes gcc warnings including:
ChProp.c: In function 'XChangeProperty':
ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
ChProp.c:83:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetHints.c: In function 'XSetStandardProperties':
SetHints.c:262:20: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetPntMap.c: In function 'XSetPointerMapping':
SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StBytes.c: In function 'XStoreBuffer':
StBytes.c:97:33: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StName.c: In function 'XStoreName':
StName.c:40:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
StName.c: In function 'XSetIconName':
StName.c:51:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The final } matches the one on the #define line, not one that doesn't
appear after the else statement it was lined up with
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
GetEmptyReq and GetResReq cannot do this due to the final typecast -
typically requests that need either of those do not have their own typedef
in the protocol headers.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Some XI2 requests change in size over different versions and libXi would
need to hack around GetReq and GetReqExtra. Add a new GetReqSized so the
library can explicitly specify the size of the request in 4-byte units.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
IsModifierKey, defined in include/X11/Xutil.h, is a macro determining,
which keys are regarded as modifiers. The constants ISO_Level5_Shift,
ISO_Level5_Latch and ISO_Level5_Lock where excluded previously, leaving
some Neo2 modifiers functionless in combination with compose.
This patch adjusts the range to include the correct, full range of
modifier constants.
Neo2 Bug 277 <http://wiki.neo-layout.org/ticket/277>
X.Org Bug 21910 <http://bugs.freedesktop.org/show_bug.cgi?id=21910>
Signed-off-by: Bodo Graumann <mail@bodograumann.de>
Reviewed-by: Daniel Stone <daniel@fooishbar.org>
This provides a simplified version of the SetReqLen macro when using clang for
static analysis. Prior to this change, we would see many Idempotent operation
warnings inside this macro due to the common case of calling with arg2 and
arg3 being the same variable. This has no effect on code produced during
compilation, but it silences a number of false positives in static analysis.
XIPassiveGrab.c:170:5: warning: Assigned value is always the same as the existing value
SetReqLen(req, num_modifiers, num_modifiers);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from XIPassiveGrab.c:26:
.../include/X11/Xlibint.h:580:8: note: instantiated from:
n = badlen; \
^
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
This will prevent a number of false positives in where clang's
static analysis reports about calls to malloc(0).
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
This reverts commit c870111546.
The constification of a pointer to a pointer caused unexpected issues,
and xorg-devel was unable to come up with a clean, safe, reasonable way
to handle them, so we're chalking this up for now as yet another mistake
in the Xlib API definition we'll be living with.
See https://bugs.freedesktop.org/show_bug.cgi?id=32098 for details.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Updates code & docs for XInternAtoms.
The single atom name argument to XInternAtom was already const char *
in the code, but not the docs, so updated it in the docs too.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
This addresses a build failure which can result from <X11/Xlocale.h> and
<xlocale.h> being included in the same code since they both used the same
_XLOCALE_H_ protection.
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
Inspired by a pattern in NoMachine's NX. Consistently zeroed buffers
compress better with ssh and friends. Note that you'll need to rebuild
all your protocol libraries to take advantage of this.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
And there was much rejoicing.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Consensus on #xorg-devel agrees with removing --without-xcb; in
particular, acks from Adam Jackson, Daniel Stone, Kristian Høgsberg,
Julien Cristau, and Rémi Cardona.
imLcIm.c: In function '_XimCachedFileName':
imLcIm.c:361: warning: format '%03x' expects type 'unsigned int', but argument 8 has type 'long unsigned int'
imLcIm.c:364: warning: format '%03x' expects type 'unsigned int', but argument 8 has type 'long unsigned int'
imRm.c: In function '_XimDefaultArea':
imRm.c:597: warning: cast from pointer to integer of different size
imRm.c: In function '_XimDefaultColormap':
imRm.c:626: warning: cast from pointer to integer of different size
lcFile.c:224: warning: no previous prototype for 'xlocaledir'
lcUTF8.c: In function 'iconv_cstombs':
lcUTF8.c:1841: warning: assignment discards qualifiers from pointer target type
lcUTF8.c:1869: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness
lcUTF8.c:1873: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness
lcUTF8.c: In function 'iconv_mbstocs':
lcUTF8.c:1935: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness
lcUTF8.c: In function 'iconv_mbtocs':
lcUTF8.c:2031: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness
lcUTF8.c: In function 'iconv_mbstostr':
lcUTF8.c:2121: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness
lcUTF8.c: In function 'iconv_strtombs':
lcUTF8.c:2180: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness
lcUTF8.c: In function '_XlcAddGB18030LocaleConverters':
lcUTF8.c:2367: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2368: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2373: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2374: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2375: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2376: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
lcUTF8.c:2377: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type
XlibInt.c: In function '_XGetHostname':
XlibInt.c:3441: warning: implicit declaration of function 'gethostname'
XlibInt.c:3441: warning: nested extern declaration of 'gethostname'
ConnDis.c: In function '_XDisconnectDisplay':
ConnDis.c:540: warning: old-style function definition
ConnDis.c: In function '_XSendClientPrefix':
ConnDis.c:554: warning: old-style function definition
ConnDis.c: In function 'XSetAuthorization':
ConnDis.c:677: warning: old-style function definition
Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
Using common defaults will reduce errors and maintenance.
Only the very small or inexistent custom section need periodic maintenance
when the structure of the component changes. Do not edit defaults.
Generic events require more bytes than Xlib provides in the standard XEvent.
Memory allocated by the extension and stored as pointers inside the event is
prone to leak by simple 'while (1) { XNextEvent(...); }' loops.
This patch adds cookie handling for generic events. Extensions may register
a cookie handler in addition to the normal event vectors. If an extension
has registered a cookie handler, _all_ generic events for this extensions
must be handled through cookies. Otherwise, the default event handler is
used.
The cookie handler must return an XGenericEventCookie with a pointer to the
data.The rest of the event (type, serialNumber, etc.) are to be filled as
normal. When a client retrieves such a cookie event, the data is stored in
an internal queue (the 'cookiejar'). This data is freed on the next call to
XNextEvent().
New extension interfaces:
XESetWireToEventCookie(display, extension_number, cookie_handler)
Where cookie_handler must set cookie->data. The data pointer is of arbitray
size and type but must be a single memory block. This memory block
represents the actual extension's event.
New client interfaces:
XGetEventData(display, *cookie);
XFreeEventData(display, *cookie);
If the client needs the actual event data, it must call XGetEventData() with
the cookie. This returns the data pointer (and removes it from the cookie
jar) and the client is then responsible for freeing the event with
XFreeEventData(). It is safe to call either function with a non-cookie
event. Events unclaimed or not handled by the XGetEventData() are cleaned up
automatically.
Example client code:
XEvent event;
XGenericEventCookie *cookie = &ev;
XNextEvent(display, &event);
if (XGetEventData(display, cookie)) {
XIEvent *xievent = cookie->data;
...
} else if (cookie->type == GenericEvent) {
/* handle generic event */
} else {
/* handle extension/core event */
}
XFreeEventData(display, cookie);
Cookies are not multi-threading safe. Clients that use XGetEventData() must
lock between XNextEvent and XGetEventData to avoid other threads freeing
cookies.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
