Changes to XInput Proto Number of Events Cause Xlib WireToEvent Vector Mismatch
Peter Hutterer
peter.hutterer at who-t.net
Wed Nov 25 17:45:39 PST 2009
On Wed, Nov 25, 2009 at 04:42:59PM -0500, Nathan Kidd wrote:
> In the last few years inputproto's number of events (IEVENTS) has jumped
> around quite a bit between 15 and 19, which has resulted in the
> following issue I've recently became aware of:
>
> E.g.
>
> 1. xserver is built from inputproto with IEVENTS as 15 (e.g. SLES 10)
> 15 is statically compiled into xserver.
> 2. libXi is built some time later with IEVENTS as 17 (e.g. Fedora 11)
> 17 is statically compiled into libXi.
> 3. xclient runs on machine with new libXi
> xclient -> QueryExtension(XKEYBOARD)
> xserver <- event base 100
> xlib sets event_vec[100] to an xkeyboard WireToEvent function
> xclient -> QueryExtension(XInputExtension)
> xserver <- event base 85
> xlib sets event_vec[85 .. 85 + 17] xinput WireToEvent function, i.e.
> OVERWRITES xkeyboard's WireToEvent proc because xserver only allowed for
> 15 events between the event bases, since that's all it knew about when
> it was built.
> 4. the wrong WireToEvent function get's used on subsequent XKEYBOARD events
Thanks for the detailed analysis, it helped greatly scoping out a possible
fix for this issue. AIUI, other extensions may be affected as well, not just
Xi. Any newer library extension run against an older server (where the
extension differs in the number of events) would suffer from this issue.
Attached is an attempt of a fix to libXext. There's not a lot of
wriggle-room, the APIs are quite restrictive and we can't pass a great deal
of information around. Additionally, the library has no way of knowing how
many events a given extension has, it's pure guesswork (or compiled into the
library).
I'll let the patch stand as-is, there's a long comment essentially
stating your findings and a description of the fix. A second attachment is a
simple test program to verify the bug and the fix. Run with LD_PRELOAD set
libfakelib.so.
Cheers,
Peter
-------------- next part --------------
>From ef1abf3fc6a0aa8b47014df078fe551f673142e5 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer at who-t.net>
Date: Thu, 26 Nov 2009 09:38:31 +1000
Subject: [PATCH] Don't smash the event_vec if num_events differs between lib and server.
If the library extension thinks there's more events to an extension than the
server actually has, the event_vec for the overlapping range can get
overwritten. This depends on the initialization order of the libraries.
Reported-by: Nathan Kidd
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
src/extutil.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 43 insertions(+), 1 deletions(-)
diff --git a/src/extutil.c b/src/extutil.c
index ac861ea..ff823bd 100644
--- a/src/extutil.c
+++ b/src/extutil.c
@@ -103,6 +103,7 @@ XExtDisplayInfo *XextAddDisplay (
int nevents,
XPointer data)
{
+ static unsigned char ext_handlers[64] = {0};
XExtDisplayInfo *dpyinfo;
dpyinfo = (XExtDisplayInfo *) Xmalloc (sizeof (XExtDisplayInfo));
@@ -117,10 +118,51 @@ XExtDisplayInfo *XextAddDisplay (
*/
if (dpyinfo->codes) {
int i, j;
+ int idx = dpyinfo->codes->first_event & 0x3f;
+
+
+ /* Xlib extensions use compiled in event numbers. A new library
+ * against an older server may thus expect a different (higher)
+ * number of events than the server will send. We have no way of
+ * knowing the number of events for an extension, the server won't
+ * tell us.
+ *
+ * Depending on the extension initialization order, this smashes the
+ * event_vec[type] for anything after the extension with the
+ * different number of events.
+ *
+ * e.g. server with inputproto 1.3 expects 15 events, libXi with
+ * inputproto 2.0 expects 17 events.
+ * base code is 80, events [80,96] are handled by libXi. events [95,
+ * 96] belong to the next extension already though.
+ * This requires XI to be initialized after the extension occupying
+ * the next range of event codes.
+ *
+ * To avoid this, we have a zeroed out array of extension handlers.
+ * If an extension handler for an event type is already set, and the
+ * previous event code (before base_code) is the same extension, we
+ * have the nevents conflict. Unset all those handlers and allow
+ * overwriting them with the new handlers.
+ *
+ * If a handler for a (base + n) event is already set, stop
+ * registering this extension for the event codes.
+ *
+ * event_codes are subtracted by 64 since we don't need to worry
+ * about core.
+ */
+
+ if (idx && ext_handlers[idx - 1] == ext_handlers[idx]) {
+ for (i = idx; i < idx + nevents; i++)
+ if (ext_handlers[idx - 1] == ext_handlers[i])
+ ext_handlers[i] = 0;
+ }
- for (i = 0, j = dpyinfo->codes->first_event; i < nevents; i++, j++) {
+ for (i = 0, j = dpyinfo->codes->first_event; i < nevents; i++, j++, idx++) {
+ if (ext_handlers[idx]) /* don't smash the following extension */
+ break;
XESetWireToEvent (dpy, j, hooks->wire_to_event);
XESetEventToWire (dpy, j, hooks->event_to_wire);
+ ext_handlers[idx] = dpyinfo->codes->first_event & 0x3f;
}
/* register extension for XGE */
--
1.6.5.2
-------------- next part --------------
>From ffacdb05748fdbedd02f98f840068bd5ce9b5250 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer at who-t.net>
Date: Thu, 26 Nov 2009 11:26:39 +1000
Subject: [PATCH] Add xext_event_smash test program.
---
xext_event_smash/Makefile.am | 9 +++++
xext_event_smash/fake_lib.c | 31 ++++++++++++++++
xext_event_smash/xext_event_smash.c | 67 +++++++++++++++++++++++++++++++++++
3 files changed, 107 insertions(+), 0 deletions(-)
create mode 100644 xext_event_smash/Makefile.am
create mode 100644 xext_event_smash/fake_lib.c
create mode 100644 xext_event_smash/xext_event_smash.c
diff --git a/xext_event_smash/Makefile.am b/xext_event_smash/Makefile.am
new file mode 100644
index 0000000..ee0402b
--- /dev/null
+++ b/xext_event_smash/Makefile.am
@@ -0,0 +1,9 @@
+bin_PROGRAMS=xext_event_smash
+lib_LTLIBRARIES = libfakelib.la
+
+AM_CFLAGS=-ggdb -Wall
+xext_event_smash_LDADD=$(X_LIBS)
+
+libfakelib_la_SOURCES = fake_lib.c
+libfakelib_la_LIBADD = $(X_LIBS)
+
diff --git a/xext_event_smash/fake_lib.c b/xext_event_smash/fake_lib.c
new file mode 100644
index 0000000..33873f2
--- /dev/null
+++ b/xext_event_smash/fake_lib.c
@@ -0,0 +1,31 @@
+#include <X11/Xlibint.h>
+#include <string.h>
+#define __USE_GNU
+#include <dlfcn.h>
+
+XExtCodes *XInitExtension(Display *dpy, const char *name) {
+ XExtCodes codes;
+ _XExtension *ext;
+ static XExtCodes *(*real_func)(Display*, const char *);
+
+ if (!real_func)
+ real_func = dlsym(RTLD_NEXT, "XInitExtension");
+
+ if (strcmp(name, "foo") == 0)
+ {
+ codes.major_opcode = 1;
+ codes.first_event = 89;
+ codes.first_error = 1;
+ } else if (strcmp(name, "bar") == 0)
+ {
+ codes.major_opcode = 2;
+ codes.first_event = 96;
+ codes.first_error = 2;
+ } else
+ return real_func(dpy, name);
+
+ ext = calloc(1, sizeof(_XExtension));
+ ext->codes = codes;
+
+ return &ext->codes;
+}
diff --git a/xext_event_smash/xext_event_smash.c b/xext_event_smash/xext_event_smash.c
new file mode 100644
index 0000000..9900b7f
--- /dev/null
+++ b/xext_event_smash/xext_event_smash.c
@@ -0,0 +1,67 @@
+#include <assert.h>
+
+#include <X11/Xlib.h>
+#include <X11/Xlibint.h>
+#include <X11/extensions/extutil.h>
+#include <X11/extensions/Xext.h>
+#include <X11/extensions/XI.h>
+
+Bool wire_to_event_bar(Display *display, XEvent *re, xEvent *event) {
+ return True;
+}
+Bool wire_to_event_foo(Display *display, XEvent *re, xEvent *event) {
+ return True;
+}
+
+static XExtensionHooks hooks_bar = {
+ .wire_to_event = wire_to_event_foo
+};
+
+static XExtensionHooks hooks_foo = {
+ .wire_to_event = wire_to_event_bar
+};
+
+int main(void)
+{
+ XExtDisplayInfo *dpyinfo_foo, *dpyinfo_bar;
+ XExtensionInfo *info;
+ Display *dpy;
+ int i;
+ int first_ev_diff;
+ XExtCodes *foocodes, *barcodes;
+
+ dpy = XOpenDisplay(NULL);
+ assert(dpy);
+
+
+ info = XextCreateExtension();
+ assert(info);
+
+ /* Add extension "bar", setting the higher event codes */
+ dpyinfo_bar = XextAddDisplay(info, dpy, "bar", &hooks_foo, 25, NULL);
+ barcodes = dpyinfo_bar->codes;
+
+ /* check event vector is correct */
+ for (i = 0; i < 25; i++)
+ assert(dpy->event_vec[barcodes->first_event + i] == wire_to_event_bar);
+
+ /* Add extension "foo", setting the lower event codes */
+ dpyinfo_foo = XextAddDisplay(info, dpy, "foo", &hooks_bar, 25, NULL);
+ foocodes = dpyinfo_foo->codes;
+
+ /* check bar's event handlers didn't get messed up. */
+ for (i = 0; i < 25; i++)
+ assert(dpy->event_vec[barcodes->first_event + i] == wire_to_event_bar);
+
+ first_ev_diff = barcodes->first_event - foocodes->first_event;
+
+ /* check foo's event handlers below bar are set */
+ for (i = 0; i < first_ev_diff; i++)
+ assert(dpy->event_vec[foocodes->first_event + i] == wire_to_event_foo);
+
+ /* rest belong to bar */
+ for (i = first_ev_diff; i < 25; i++)
+ assert(dpy->event_vec[foocodes->first_event + i] == wire_to_event_bar);
+
+ return 0;
+}
--
1.6.5.2
More information about the xorg
mailing list