Backport fix for Xlib lockups due to recursive XError
Resolves: https://issues.redhat.com/browse/RHEL-23452
This commit is contained in:
parent
78f64db9c8
commit
39ea10b841
166
0001-Avoid-recursing-through-_XError-due-to-sequence-adju.patch
Normal file
166
0001-Avoid-recursing-through-_XError-due-to-sequence-adju.patch
Normal file
@ -0,0 +1,166 @@
|
|||||||
|
From 8c92ef59890c6d6e2be456658d3b9c145eda8629 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Keith Packard <keithp@keithp.com>
|
||||||
|
Date: Sat, 7 Nov 2020 22:22:47 -0800
|
||||||
|
Subject: [PATCH libX11] 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>
|
||||||
|
(cherry picked from commit 30ccef3a48029bf4fc31d4abda2d2778d0ad6277)
|
||||||
|
---
|
||||||
|
include/X11/Xlibint.h | 2 ++
|
||||||
|
src/OpenDis.c | 1 +
|
||||||
|
src/XlibInt.c | 10 ++++++++++
|
||||||
|
src/locking.c | 12 ++++++++++++
|
||||||
|
src/locking.h | 12 ++++++++++++
|
||||||
|
5 files changed, 37 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/include/X11/Xlibint.h b/include/X11/Xlibint.h
|
||||||
|
index 6b95bcf7..09078e3f 100644
|
||||||
|
--- a/include/X11/Xlibint.h
|
||||||
|
+++ b/include/X11/Xlibint.h
|
||||||
|
@@ -202,6 +202,8 @@ struct _XDisplay
|
||||||
|
unsigned long last_request_read_upper32bit;
|
||||||
|
unsigned long request_upper32bit;
|
||||||
|
#endif
|
||||||
|
+
|
||||||
|
+ struct _XErrorThreadInfo *error_threads;
|
||||||
|
};
|
||||||
|
|
||||||
|
#define XAllocIDs(dpy,ids,n) (*(dpy)->idlist_alloc)(dpy,ids,n)
|
||||||
|
diff --git a/src/OpenDis.c b/src/OpenDis.c
|
||||||
|
index 82723578..85901168 100644
|
||||||
|
--- a/src/OpenDis.c
|
||||||
|
+++ b/src/OpenDis.c
|
||||||
|
@@ -201,6 +201,7 @@ XOpenDisplay (
|
||||||
|
X_DPY_SET_LAST_REQUEST_READ(dpy, 0);
|
||||||
|
dpy->default_screen = iscreen; /* Value returned by ConnectDisplay */
|
||||||
|
dpy->last_req = (char *)&_dummy_request;
|
||||||
|
+ dpy->error_threads = NULL;
|
||||||
|
|
||||||
|
/* Initialize the display lock */
|
||||||
|
if (InitDisplayLock(dpy) != 0) {
|
||||||
|
diff --git a/src/XlibInt.c b/src/XlibInt.c
|
||||||
|
index 4e45e62b..8771b791 100644
|
||||||
|
--- a/src/XlibInt.c
|
||||||
|
+++ b/src/XlibInt.c
|
||||||
|
@@ -1482,6 +1482,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);
|
||||||
|
@@ -1491,6 +1496,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 {
|
||||||
|
diff --git a/src/locking.c b/src/locking.c
|
||||||
|
index 9f4fe067..bcadc857 100644
|
||||||
|
--- a/src/locking.c
|
||||||
|
+++ b/src/locking.c
|
||||||
|
@@ -453,6 +453,9 @@ static void _XLockDisplay(
|
||||||
|
XTHREADS_FILE_LINE_ARGS
|
||||||
|
)
|
||||||
|
{
|
||||||
|
+#ifdef XTHREADS
|
||||||
|
+ struct _XErrorThreadInfo *ti;
|
||||||
|
+#endif
|
||||||
|
#ifdef XTHREADS_WARN
|
||||||
|
_XLockDisplayWarn(dpy, file, line);
|
||||||
|
#else
|
||||||
|
@@ -460,6 +463,15 @@ static void _XLockDisplay(
|
||||||
|
#endif
|
||||||
|
if (dpy->lock->locking_level > 0)
|
||||||
|
_XDisplayLockWait(dpy);
|
||||||
|
+#ifdef XTHREADS
|
||||||
|
+ /*
|
||||||
|
+ * Skip the two function calls below which may generate requests
|
||||||
|
+ * when LockDisplay is called from within _XError.
|
||||||
|
+ */
|
||||||
|
+ for (ti = dpy->error_threads; ti; ti = ti->next)
|
||||||
|
+ if (ti->error_thread == xthread_self())
|
||||||
|
+ return;
|
||||||
|
+#endif
|
||||||
|
_XIDHandler(dpy);
|
||||||
|
_XSeqSyncFunction(dpy);
|
||||||
|
}
|
||||||
|
diff --git a/src/locking.h b/src/locking.h
|
||||||
|
index 5251a60c..59fc866e 100644
|
||||||
|
--- a/src/locking.h
|
||||||
|
+++ b/src/locking.h
|
||||||
|
@@ -149,6 +149,18 @@ typedef struct _LockInfoRec {
|
||||||
|
xmutex_t lock;
|
||||||
|
} LockInfoRec;
|
||||||
|
|
||||||
|
+/* A list of threads currently invoking error handlers on this display
|
||||||
|
+ * LockDisplay operates differently for these threads, avoiding
|
||||||
|
+ * generating any requests or reading any events as that can cause
|
||||||
|
+ * recursion into the error handling code, which will deadlock the
|
||||||
|
+ * thread.
|
||||||
|
+ */
|
||||||
|
+struct _XErrorThreadInfo
|
||||||
|
+{
|
||||||
|
+ struct _XErrorThreadInfo *next;
|
||||||
|
+ xthread_t error_thread;
|
||||||
|
+};
|
||||||
|
+
|
||||||
|
/* XOpenDis.c */
|
||||||
|
extern int (*_XInitDisplayLock_fn)(Display *dpy);
|
||||||
|
extern void (*_XFreeDisplayLock_fn)(Display *dpy);
|
||||||
|
--
|
||||||
|
2.43.0
|
||||||
|
|
@ -5,7 +5,7 @@
|
|||||||
Summary: Core X11 protocol client library
|
Summary: Core X11 protocol client library
|
||||||
Name: libX11
|
Name: libX11
|
||||||
Version: 1.6.8
|
Version: 1.6.8
|
||||||
Release: 7%{?gitdate:.%{gitdate}git%{gitversion}}%{?dist}
|
Release: 8%{?gitdate:.%{gitdate}git%{gitversion}}%{?dist}
|
||||||
License: MIT
|
License: MIT
|
||||||
Group: System Environment/Libraries
|
Group: System Environment/Libraries
|
||||||
URL: http://www.x.org
|
URL: http://www.x.org
|
||||||
@ -39,6 +39,9 @@ Patch11: 0003-XCreatePixmap-trigger-BadValue-error-for-out-of-rang.patch
|
|||||||
# CVE-2023-43787
|
# CVE-2023-43787
|
||||||
Patch12: 0001-CVE-2023-43787-Integer-overflow-in-XCreateImage-lead.patch
|
Patch12: 0001-CVE-2023-43787-Integer-overflow-in-XCreateImage-lead.patch
|
||||||
|
|
||||||
|
# RHEL-23452
|
||||||
|
Patch13: 0001-Avoid-recursing-through-_XError-due-to-sequence-adju.patch
|
||||||
|
|
||||||
BuildRequires: xorg-x11-util-macros >= 1.11
|
BuildRequires: xorg-x11-util-macros >= 1.11
|
||||||
BuildRequires: pkgconfig(xproto) >= 7.0.15
|
BuildRequires: pkgconfig(xproto) >= 7.0.15
|
||||||
BuildRequires: xorg-x11-xtrans-devel >= 1.0.3-4
|
BuildRequires: xorg-x11-xtrans-devel >= 1.0.3-4
|
||||||
@ -89,6 +92,7 @@ libX11/libxcb interoperability library
|
|||||||
%patch10 -p1 -b .xputimage-clip-images-to-maximum-height-width-allowe
|
%patch10 -p1 -b .xputimage-clip-images-to-maximum-height-width-allowe
|
||||||
%patch11 -p1 -b .xcreatepixmap-trigger-badvalue-error-for-out-of-rang
|
%patch11 -p1 -b .xcreatepixmap-trigger-badvalue-error-for-out-of-rang
|
||||||
%patch12 -p1 -b .cve-2023-43787
|
%patch12 -p1 -b .cve-2023-43787
|
||||||
|
%patch13 -p1 -b .rhel-23452
|
||||||
|
|
||||||
%build
|
%build
|
||||||
autoreconf -v --install --force
|
autoreconf -v --install --force
|
||||||
@ -153,6 +157,9 @@ make %{?_smp_mflags} check
|
|||||||
%{_mandir}/man5/*.5*
|
%{_mandir}/man5/*.5*
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Tue Jan 30 2024 Olivier Fourdan <ofourdan@redhat.com> - 1.6.8-8
|
||||||
|
- Backport fix for Xlib lockups due to recursive XError (RHEL-23452)
|
||||||
|
|
||||||
* Wed Oct 11 2023 José Expósito <jexposit@redhat.com> - 1.6.8-7
|
* Wed Oct 11 2023 José Expósito <jexposit@redhat.com> - 1.6.8-7
|
||||||
- Fix CVE-2023-43785: out-of-bounds memory access in _XkbReadKeySyms()
|
- Fix CVE-2023-43785: out-of-bounds memory access in _XkbReadKeySyms()
|
||||||
- Fix CVE-2023-43786: stack exhaustion from infinite recursion in
|
- Fix CVE-2023-43786: stack exhaustion from infinite recursion in
|
||||||
|
Loading…
Reference in New Issue
Block a user