diff options
| author | Keith Packard <keithp@keithp.com> | 2020-11-07 22:22:47 -0800 |
|---|---|---|
| committer | Keith Packard <keithp@keithp.com> | 2020-11-15 10:09:16 -0800 |
| commit | 30ccef3a48029bf4fc31d4abda2d2778d0ad6277 (patch) | |
| tree | 728256e75ad588f2b5a7deb64069b4f7d0760092 /src/XlibInt.c | |
| parent | c9c4d6efbf92ab51695e2e740319503221d68eed (diff) | |
| download | xorg-lib-libX11-30ccef3a48029bf4fc31d4abda2d2778d0ad6277.tar.gz | |
Avoid recursing through _XError due to sequence adjustment
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>
Diffstat (limited to 'src/XlibInt.c')
| -rw-r--r-- | src/XlibInt.c | 14 |
1 files changed, 11 insertions, 3 deletions
diff --git a/src/XlibInt.c b/src/XlibInt.c index a316dba3..93f0118b 100644 --- a/src/XlibInt.c +++ b/src/XlibInt.c @@ -218,12 +218,10 @@ void _XSeqSyncFunction( xGetInputFocusReply rep; _X_UNUSED register xReq *req; - if ((X_DPY_GET_REQUEST(dpy) - X_DPY_GET_LAST_REQUEST_READ(dpy)) >= (65535 - BUFSIZE/SIZEOF(xReq)) && !dpy->req_seq_syncing) { - dpy->req_seq_syncing = True; + if ((X_DPY_GET_REQUEST(dpy) - X_DPY_GET_LAST_REQUEST_READ(dpy)) >= (65535 - BUFSIZE/SIZEOF(xReq))) { GetEmptyReq(GetInputFocus, req); (void) _XReply (dpy, (xReply *)&rep, 0, xTrue); sync_while_locked(dpy); - dpy->req_seq_syncing = False; } else if (sync_hazard(dpy)) _XSetPrivSyncFunction(dpy); } @@ -1494,6 +1492,11 @@ int _XError ( if (_XErrorFunction != NULL) { int rtn_val; #ifdef XTHREADS + struct _XErrorThreadInfo thread_info = { + .error_thread = xthread_self(), + .next = dpy->error_threads + }, **prev; + dpy->error_threads = &thread_info; if (dpy->lock) (*dpy->lock->user_lock_display)(dpy); UnlockDisplay(dpy); @@ -1503,6 +1506,11 @@ int _XError ( LockDisplay(dpy); if (dpy->lock) (*dpy->lock->user_unlock_display)(dpy); + + /* unlink thread_info from the list */ + for (prev = &dpy->error_threads; *prev != &thread_info; prev = &(*prev)->next) + ; + *prev = thread_info.next; #endif return rtn_val; } else { |