[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#661652: pu: package libxi/2:1.3-7



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: pu

Hi,

libXi has had several important fixes upstream over the past year, some
of which are required for operation with recent X servers (see
bug#660411 e.g.).  A lot of it is related to calculating the proper size
of an allocation to fill in structures with the right size and
alignment, so it's rather painful unfortunately.

Not all of these changes are in sid yet, so whatever's decided I won't
upload before that's done.  The affected interfaces are part of XI2,
which doesn't have a lot of users in squeeze (seems to be just xinput
and synaptiks), but gets a lot more exposure now with e.g. gtk3.

The proposed changes are available at
http://anonscm.debian.org/gitweb/?p=users/jcristau/libxi.git;a=shortlog;h=refs/heads/debian-squeeze

I'll give the changelog and some comments below.

> commit 7ef340dd49a0a79ce2f25823f57d25674d74df82
> Author: Michał Masłowski <mtjm@mtjm.eu>
> Date:   Tue Feb 21 20:54:40 2012 +0100
> 
>     Fix bus error on MIPS N32 for bug #38331.
>     
>     XIValuatorClassInfo and XIScrollClassInfo might have an address
>     of 4 bytes modulo 8, while they contain doubles which need 8 byte
>     alignment.  This is fixed by adding extra padding after each structure
>     or array in sizeDeviceClassType and adding helper functions to
>     determine sizes and padding only in one place.
>     
>     Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=38331
>     Signed-off-by: Michał Masłowski <mtjm@mtjm.eu>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit c1a5a70b51f12dedf354102217c7cd4247ed3a4b)
>     
>     Conflicts:
>     
>     	src/XExtInt.c
>     
>     [conflict fixed by dropping the Scroll and Touch classes, not present in
>      libXi 1.3]
> 
>  src/XExtInt.c |  127 ++++++++++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 95 insertions(+), 32 deletions(-)
> 
Probably affects sparc as well.  I think this is #636920, will need to
confirm.

> commit d5dbca39dfdf666226b6bb2dc41509d6a859e51b
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Tue Jan 17 21:26:14 2012 +0100
> 
>     Force class alignment to a multiple of sizeof(XID).
>     
>     Calculate length field to a multiples of sizeof(XID). XIDs are typedefs
>     to ulong and thus may be 8 bytes on some platforms. This can trigger a
>     SIGBUS if a class ends up not being 8-aligned (e.g. after XAxisInfo).
>     
>     Reported-by: Nicolai Stange <nicolai.stange@zmaw.de>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Signed-off-by: Matthieu Herrb <matthieu.herrb@laas.fr>
>     (cherry picked from commit 07ced7b48219e3bc0c98806f3d7106f86d1b2ca0)
> 
>  src/XListDev.c |   27 +++++++++++++++++++--------
>  1 files changed, 19 insertions(+), 8 deletions(-)
> 
Affects 64bit archs with strict alignment requirement.  I *think*
there's none of those in squeeze.

> commit 088f20c3ae4d8d8b3688bf2ab7ee2ea9530f7fe2
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Oct 26 08:55:35 2011 +1000
> 
>     Stop unnecessary calls to size_classes
>     
>     Xmalloc is a macro evaluating its argument twice.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     (cherry picked from commit ba83a1e58592e41f207524f106ba26dd71fe4171)
> 
>  src/XIQueryDevice.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> commit b409d3a60e5c0ff9ca6312de19ed1ce2871532ac
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Sep 28 11:16:42 2011 +1000
> 
>     Remove superfluous assignment of lib->classes in XIQueryDevices.
>     
>     A few lines south from here we malloc lib->classes, this assignment is a
>     leftover from 225071e2e67fb65a0258397212f9826c9b25e078.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Chase Douglas <chase.douglas@canonical.com>
>     (cherry picked from commit 7ca05f3094958c04e8f78a786061124c58f8e1f3)
> 
>  src/XIQueryDevice.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> commit be149a530c5ce1d4094ed361da3208476202d4b4
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Fri Sep 23 09:04:21 2011 +1000
> 
>     Use a separate nclasses variable in XIQueryDevice
>     
>     No functional changes, just clarifying the code. If we skip over unknown
>     classes, lib->num_classes != wire->num_classe. Use a separate variable to
>     make that change more explicit and align the code closer with
>     wireToDeviceChangedEvent.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Daniel Stone <daniel@fooishbar.org>
>     (cherry picked from commit 5f9df47340e192d095127e3c7da180b0fb3dc286)
> 
>  src/XIQueryDevice.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
Those three should be straightforward, and reduce the risk of conflicts
with other cherry-picks.

> commit 84e78db42317e34f0b7880e76b32269333fc824a
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Oct 26 08:54:53 2011 +1000
> 
>     Fix duplicate sizeof in copy_classes
>     
>     sizeof(bla * sizeof()) is'nt right.
>     
>     Plus add some () to the next_block call too to emphasise that *nclasses is
>     the multiplicator.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     (cherry picked from commit 6d6ae8fc8b9620bf864ac7dff8d818573eee3e4f)
> 
>  src/XExtInt.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
Initializes a buffer correctly...

> commit d33c64350c961e3b096565b30e1e98f7c9a569dc
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Aug 17 15:20:48 2011 +1000
> 
>     Handle unknown device classes.
>     
>     If the server sends an unknown device class in response to an XIQueryDevice
>     call, no memory is allocated for these classes but we still write type
>     and sourceid as well as setting to->classes[i]. The latter causes multiple
>     classes to point to the same memory field.
>     
>     Move the common code of assigning these three into the respective class type
>     handlers so to automatically skip any unknown classes.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit 91f928a70246c26cbee00bf59a8e318e9317142e)
> 
>  src/XExtInt.c       |   30 +++++++++++++++++++++---------
>  src/XIQueryDevice.c |    4 ++--
>  2 files changed, 23 insertions(+), 11 deletions(-)
> 
This one avoids memory corruption and was reported as #660411.

> commit 26ed064a26f233a5da6c369c6bf2c3b21112a022
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Fri Aug 12 14:24:26 2011 +1000
> 
>     Don't use the protocol defines for 2.0 versioning.
>     
>     Otherwise we run into the old problem again: recompiling libXi against
>     newer inputproto headers will appear to change the version support,
>     potentially causing errors or other misbehaviours.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     (cherry picked from commit ca73cd3b7630e7eb7d26c61c4af10d35dbce5465)
> 
>  src/XExtInt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
No change for squeeze, but makes things safer (against building in the
wrong environment, or against x11proto-input from bpo, e.g.).

> commit 7289946d9e79deb7dcc3bf5a6223a026bac243a1
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Thu Jun 2 16:21:59 2011 +1000
> 
>     Use Data, not Data32 in XIPassiveGrabDevice
>     
>     Data32 takes and iterates over an array of longs, thus skipping every 4
>     bytes on LP64. Here we only have arrays of ints, use the normal Data macro
>     instead.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     (cherry picked from commit 9faab2bc0bdd4d98a04e572a7a5201bfcd3bdc70)
> 
>  src/XIPassiveGrab.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
Xlib wackiness strikes again.  This avoids reading from outside
the buffer and sending garbage to the server, AFAICT.

> commit d6cec8b76e4d99fbc9f8ed499adb7cd9976a2721
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Tue May 3 14:34:22 2011 +1000
> 
>     XIChangeHierarchy: Return Success early if no actual changes are requested.
>     
>     Do the same for negative num_changes.
>     
>     Found by static analyzer.
>     
>     Reported-by: Jeremy Huddleston <jeremyhu@apple.com>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     (cherry picked from commit cc6feecae23b321952921cf927bc965386844c8c)
> 
>  man/XIChangeHierarchy.txt |    3 ++-
>  src/XIHierarchy.c         |    3 +++
>  2 files changed, 5 insertions(+), 1 deletions(-)
> 
Hopefully nothing relies on calling this with a negative arg, so
possibly superfluous.

> commit 67fa98eebb5016ba1f80452fafb149f3fda84d4b
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Tue May 3 13:21:02 2011 +1000
> 
>     Allocate enough memory for raw events + extra data.
>     
>     Necessary space was calculated, but not actually used to allocate memory. As
>     a result, valuator data would overwrite the allocated memory.
>     
>     ==4166== Invalid write of size 1
>     ==4166==    at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
>     ==4166==    by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
>     ==4166==    by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>     ==4166==    by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>     ==4166==    by 0x49C3E3: process_key (x11_be.c:1065)
>     ==4166==    by 0x49EA5C: event_key_release (x11_be.c:2201)
>     ==4166==    by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>     ==4166==    by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>     ==4166==    by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>     ==4166==    by 0x87549C9: start_thread (pthread_create.c:300)
>     ==4166==    by 0x8A516FC: clone (clone.S:112)
>     ==4166==  Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
>     ==4166==    at 0x4C284A8: malloc (vg_replace_malloc.c:236)
>     ==4166==    by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
>     ==4166==    by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>     ==4166==    by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>     ==4166==    by 0x49C3E3: process_key (x11_be.c:1065)
>     ==4166==    by 0x49EA5C: event_key_release (x11_be.c:2201)
>     ==4166==    by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>     ==4166==    by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>     ==4166==    by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>     ==4166==    by 0x87549C9: start_thread (pthread_create.c:300)
>     
>     Reported-by: Roger Cruz <roger.cruz@virtualcomputer.com>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
>     Reviewed-by: Daniel Stone <daniel@fooishbar.org>
>     (cherry picked from commit af65475b1f6b7209750220a74aaad9968d54aaf7)
> 
>  src/XExtInt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
Should be clear enough.

> commit 59de5bb8e83074060cd65a3dd0370693ad4917e3
> Author: Matthieu Herrb <matthieu.herrb@laas.fr>
> Date:   Fri Apr 1 22:49:09 2011 +0200
> 
>     Fix XISelectEvents on 64 bits, strict alignement architectures.
>     
>     Use Data() to send the struct xXIEventMask on the wire instead of
>     Data32() which expects a pointer to a 64bits value on LP64
>     architectures.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit 18177afd4fb3934d0a9083c599fb96701eec2ad9)
> 
>  src/XISelEv.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
Once again, looks like this avoids sending random data off to the X
server, in addition to the possible crash on 64bit strict-alignment
archs.

> commit 0f03a60e9c4c8e5e4a7fc106e79befb5f2bb629e
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Tue Mar 15 10:55:10 2011 +1000
> 
>     Force alignment with sizeof(Atom) for XIButtonClassInfo
>     
>     The memory layout of an XIButtonClassInfo is
>     [struct XIButtonClassInfo][mask][labels]
>     
>     With the mask being currently 4-byte aligned and labels a list of Atoms. On
>     LP64, Atoms are 8 byte, leading to unaligned access for some mask lengths.
>     Force the alignment to be sizeof(Atom).
>     
>     Reported-by: Christian Weisgerber <naddy@mips.inka.de>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     Tested-by: Christian Weisgerber <naddy@mips.inka.de>
>     Reviewed-by: Adam Jackson <ajax@redhat.com>
>     (cherry picked from commit 2d638fc37b0dbf28e5c826f74f68ada83a8c3e2b)
> 
>  src/XExtInt.c |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> commit 2bec887ecc666a1783ef6e26b90f052b96ba907b
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Tue Mar 8 14:44:10 2011 +1000
> 
>     Don't discard extra data for passive grabs.
>     
>     Failed modifier data was discarded, causing assertions inside xcb.
>     
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit 477f922fb07eea629f16c55b0a022e836ede6d41)
> 
>  src/XIPassiveGrab.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> commit cc0cb523d65149b902cd627a7e8db9be18f2e61b
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Feb 23 10:53:21 2011 +1000
> 
>     Fix typo in comment.
>     
>     Reported-by: Julien Cristau <jcristau@debian.org>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit 536bd44c513ede7e61e112c82a80fb9197f261f7)
> 
>  src/XIGrabDevice.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> commit 7726b0198fee19c3c5821219c6924665b4643723
> Author: Peter Hutterer <peter.hutterer@who-t.net>
> Date:   Wed Feb 23 09:30:59 2011 +1000
> 
>     Fix invalid read in XIGrabDevice.
>     
>     Miscalculation of length caused Data() to memcpy too many bytes.
>     
>     ==2865== Invalid read of size 1
>     ==2865==    at 0x4A07480: memcpy (mc_replace_strmem.c:602)
>     ==2865==    by 0x544271E: XIGrabDevice (XIGrabDevice.c:69)
>     ==2865==    by 0x400B0A: main (gnome642481.c:56)
>     ==2865==  Address 0x642f614 is 0 bytes after a block of size 20 alloc'd
>     ==2865==    at 0x4A04896: calloc (vg_replace_malloc.c:418)
>     ==2865==    by 0x54425D3: XIGrabDevice (XIGrabDevice.c:65)
>     ==2865==    by 0x400B0A: main (gnome642481.c:56)
>     
>     SetReqLen() expects 4-byte units.
>     Data() expects bytes.
>     
>     Gnome Bug 642481 <https://bugzilla.gnome.org/show_bug.cgi?id=642481>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit 4ca8be9f3ffbafe9515e50d784f4ff83f6993be0)
> 
>  src/XIGrabDevice.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> commit 8f7958dbae15de702ad991eb433231f976ca6ab4
> Author: Carlos Garnacho <carlosg@gnome.org>
> Date:   Mon Jan 24 12:35:04 2011 +0100
> 
>     Fill in mods/group->effective in XIQueryPointer()
>     
>     the other XIModifierState/XIGroupState fields are being set correctly,
>     but the "effective" field was being left as undefined memory.
>     
>     Signed-off-by: Carlos Garnacho <carlosg@gnome.org>
>     Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit e0c95ce2348a9c9afaa4862368c7a5ae6913457c)
> 
>  src/XIQueryPointer.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> commit d3310f595f0eb09040e12f12bb74f83ae756706a
> Author: Philipp Reh <sefi@s-e-f-i.de>
> Date:   Mon Jan 10 17:35:57 2011 +0100
> 
>     Fix passive grabs.
>     
>     _XIPassiveGrabDevice, which is called by alle the passive grab functions,
>     wrongly returns an error when it shouldn't.
>     The attached patch adds the missing "not" to properly test the error
>     condition of _XReply.
>     
>     Signed-off-by: Philipp Reh <sefi@s-e-f-i.de>
>     Reviewed-by: Daniel Stone <daniel@fooishbar.org>
>     Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
>     (cherry picked from commit a5961a8459614fcaa801a47cda07d3ee8246b16f)
> 
>  src/XIPassiveGrab.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Hopefully these last ones are clear enough.

Finally, the combined diff for those changes.

diff --git a/debian/changelog b/debian/changelog
index 3ecdd89..7197d2b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,31 @@
+libxi (2:1.3-7) UNRELEASED; urgency=low
+
+  * Cherry-pick some patches from upstream, including various alignment issues
+    and read/write in wrong places:
+    - Fix passive grabs
+    - Fill in mods/group->effective in XIQueryPointer
+    - Fix invalid read in XIGrabDevice
+      https://bugzilla.gnome.org/show_bug.cgi?id=642481
+    - Don't discard extra data for passive grabs.
+      Failed modifier data was discarded, causing assertions inside xcb.
+    - Force alignment with sizeof(Atom) for XIButtonClassInfo
+    - Fix XISelectEvents on 64 bits, strict alignement architectures
+    - Allocate enough memory for raw events + extra data
+    - XIChangeHierarchy: Return Success early if no actual changes are
+      requested.
+    - Use Data, not Data32 in XIPassiveGrabDevice.
+    - Don't use the protocol defines for 2.0 versioning.
+    - Handle unknown device classes. (closes: #660411)
+    - Fix duplicate sizeof in copy_classes
+    - Use a separate nclasses variable in XIQueryDevice
+    - Remove superfluous assignment of lib->classes in XIQueryDevices.
+    - Stop unnecessary calls to size_classes
+    - Force class alignment to a multiple of sizeof(XID).
+    - Fix bus error on MIPS N32 for bug #38331.
+      https://bugs.freedesktop.org/show_bug.cgi?id=38331
+
+ -- Julien Cristau <jcristau@debian.org>  Tue, 08 Feb 2011 00:15:12 +0100
+
 libxi (2:1.3-6) unstable; urgency=medium
 
   * WireToEvent: Set display member of all events as well (cherry-pick from
diff --git a/man/XIChangeHierarchy.txt b/man/XIChangeHierarchy.txt
index ac667bc..205f40f 100644
--- a/man/XIChangeHierarchy.txt
+++ b/man/XIChangeHierarchy.txt
@@ -30,7 +30,8 @@ DESCRIPTION
    XIChangeHierarchy modifies the device hierarchy by creating or
    removing master devices or changing the attachment of slave
    devices. If num_changes is non-zero, changes is an array of
-   XIAnyHierarchyChangeInfo structures.
+   XIAnyHierarchyChangeInfo structures. If num_changes is equal or less than
+   zero, XIChangeHierarchy does nothing.
 
    XIChangeHierarchy processes changes in order, effective
    immediately. If an error occurs, processing is aborted and the
diff --git a/src/XExtInt.c b/src/XExtInt.c
index 4b7154b..aca5674 100644
--- a/src/XExtInt.c
+++ b/src/XExtInt.c
@@ -100,7 +100,7 @@ extern int _XiGetDevicePresenceNotifyEvent(
     Display *		/* dpy */
 );
 
-int copy_classes(XIDeviceInfo *to, xXIAnyInfo* from, int nclasses);
+int copy_classes(XIDeviceInfo *to, xXIAnyInfo* from, int *nclasses);
 int size_classes(xXIAnyInfo* from, int nclasses);
 
 static XExtensionInfo *xinput_info;
@@ -270,7 +270,7 @@ static XExtensionVersion versions[] = { {XI_Absent, 0, 0},
  XI_Add_DevicePresenceNotify_Minor},
 {XI_Present, XI_Add_DeviceProperties_Major,
  XI_Add_DeviceProperties_Minor},
-{XI_Present, XI_2_Major, XI_2_Minor}
+{XI_Present, 2, 0}
 };
 
 /***********************************************************************
@@ -1014,6 +1014,55 @@ sizeDeviceEvent(int buttons_len, int valuators_len,
     return len;
 }
 
+/* Return the size with added padding so next element would be
+   double-aligned unless the architecture is known to allow unaligned
+   data accesses.  Not doing this can cause a bus error on
+   MIPS N32. */
+static int
+pad_to_double(int size)
+{
+#if !defined(__i386__) && !defined(__sh__)
+    if (size % sizeof(double) != 0)
+        size += sizeof(double) - size % sizeof(double);
+#endif
+    return size;
+}
+
+/**
+ * Set structure and atoms to size in bytes of XIButtonClassInfo, its
+ * button state mask and labels array.
+ */
+static void
+sizeXIButtonClassType(int num_buttons, int* structure, int* state, int* atoms)
+{
+    int size;
+    int labels;
+
+    *structure = pad_to_double(sizeof(XIButtonClassInfo));
+    size = ((((num_buttons + 7)/8) + 3)/4);
+
+    /* Force mask alignment with longs to avoid unaligned
+     * access when accessing the atoms. */
+    *state = pad_to_double(size * 4);
+    labels = num_buttons * sizeof(Atom);
+
+    /* Force mask alignment with longs to avoid
+     * unaligned access when accessing the atoms. */
+    labels += ((((num_buttons + 7)/8) + 3)/4) * sizeof(Atom);
+    *atoms = pad_to_double(labels);
+}
+
+/**
+ * Set structure and keycodes to size in bytes of XIKeyClassInfo and
+ * its keycodes array.
+ */
+static void
+sizeXIKeyClassType(int num_keycodes, int* structure, int* keycodes)
+{
+    *structure = pad_to_double(sizeof(XIKeyClassInfo));
+    *keycodes = pad_to_double(num_keycodes * sizeof(int));
+}
+
 /**
  * Return the size in bytes required to store the matching class type
  * num_elements is num_buttons for XIButtonClass or num_keycodes for
@@ -1025,19 +1074,20 @@ static int
 sizeDeviceClassType(int type, int num_elements)
 {
     int l = 0;
+    int extra1 = 0;
+    int extra2 = 0;
     switch(type)
     {
         case XIButtonClass:
-            l = sizeof(XIButtonClassInfo);
-            l += num_elements * sizeof(Atom);
-            l += ((((num_elements + 7)/8) + 3)/4) * 4;
+            sizeXIButtonClassType(num_elements, &l, &extra1, &extra2);
+            l += extra1 + extra2;
             break;
         case XIKeyClass:
-            l = sizeof(XIKeyClassInfo);
-            l += num_elements * sizeof(int);
+            sizeXIKeyClassType(num_elements, &l, &extra1);
+            l += extra1;
             break;
         case XIValuatorClass:
-            l = sizeof(XIValuatorClassInfo);
+            l = pad_to_double(sizeof(XIValuatorClassInfo));
             break;
         default:
             printf("sizeDeviceClassType: unknown type %d\n", type);
@@ -1123,16 +1173,21 @@ copyDeviceChangedEvent(XGenericEventCookie *in_cookie,
         {
             case XIButtonClass:
                 {
+                    int struct_size;
+                    int state_size;
+                    int labels_size;
                     XIButtonClassInfo *bin, *bout;
                     bin = (XIButtonClassInfo*)any;
-                    bout = next_block(&ptr, sizeof(XIButtonClass));
+                    sizeXIButtonClassType(bin->num_buttons, &struct_size,
+                                          &state_size, &labels_size);
+                    bout = next_block(&ptr, struct_size);
 
                     *bout = *bin;
-                    bout->state.mask = next_block(&ptr, bout->state.mask_len);
+                    bout->state.mask = next_block(&ptr, state_size);
                     memcpy(bout->state.mask, bin->state.mask,
                             bout->state.mask_len);
 
-                    bout->labels = next_block(&ptr, bout->num_buttons * sizeof(Atom));
+                    bout->labels = next_block(&ptr, labels_size);
                     memcpy(bout->labels, bin->labels, bout->num_buttons * sizeof(Atom));
                     out->classes[i] = (XIAnyClassInfo*)bout;
                     break;
@@ -1140,11 +1195,15 @@ copyDeviceChangedEvent(XGenericEventCookie *in_cookie,
             case XIKeyClass:
                 {
                     XIKeyClassInfo *kin, *kout;
+                    int struct_size;
+                    int keycodes_size;
                     kin = (XIKeyClassInfo*)any;
+                    sizeXIKeyClassType(kin->num_keycodes, &struct_size,
+                                       &keycodes_size);
 
-                    kout = next_block(&ptr, sizeof(XIKeyClass));
+                    kout = next_block(&ptr, struct_size);
                     *kout = *kin;
-                    kout->keycodes = next_block(&ptr, kout->num_keycodes * sizeof(int));
+                    kout->keycodes = next_block(&ptr, keycodes_size);
                     memcpy(kout->keycodes, kin->keycodes, kout->num_keycodes * sizeof(int));
                     out->classes[i] = (XIAnyClassInfo*)kout;
                     break;
@@ -1153,7 +1212,8 @@ copyDeviceChangedEvent(XGenericEventCookie *in_cookie,
                 {
                     XIValuatorClassInfo *vin, *vout;
                     vin = (XIValuatorClassInfo*)any;
-                    vout = next_block(&ptr, sizeof(XIValuatorClass));
+                    vout = next_block(&ptr,
+                                      sizeDeviceClassType(XIValuatorClass, 0));
                     *vout = *vin;
                     out->classes[i] = (XIAnyClassInfo*)vout;
                     break;
@@ -1255,7 +1315,7 @@ copyRawEvent(XGenericEventCookie *cookie_in,
     len = sizeof(XIRawEvent) + in->valuators.mask_len;
     len += bits * sizeof(double) * 2;
 
-    ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
+    ptr = cookie_out->data = malloc(len);
     if (!ptr)
         return False;
 
@@ -1410,7 +1470,8 @@ size_classes(xXIAnyInfo* from, int nclasses)
     xXIAnyInfo *any_wire;
     char *ptr_wire;
 
-    len = nclasses * sizeof(XIAnyClassInfo*); /* len for to->classes */
+    /* len for to->classes */
+    len = pad_to_double(nclasses * sizeof(XIAnyClassInfo*));
     ptr_wire = (char*)from;
     for (i = 0; i < nclasses; i++)
     {
@@ -1445,30 +1506,30 @@ size_classes(xXIAnyInfo* from, int nclasses)
  *             |______________________^
  */
 _X_HIDDEN int
-copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int nclasses)
+copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int *nclasses)
 {
     XIAnyClassInfo *any_lib;
     xXIAnyInfo *any_wire;
     void *ptr_lib;
     char *ptr_wire;
     int i, len;
+    int cls_idx = 0;
 
     if (!to->classes)
         return -1;
 
     ptr_wire = (char*)from;
     ptr_lib = to->classes;
-    to->classes = next_block(&ptr_lib, nclasses * sizeof(XIAnyClassInfo*));
+    to->classes = next_block(&ptr_lib,
+                             pad_to_double((*nclasses) * sizeof(XIAnyClassInfo*)));
+    memset(to->classes, 0, (*nclasses) * sizeof(XIAnyClassInfo*));
     len = 0; /* count wire length */
 
-    for (i = 0; i < nclasses; i++)
+    for (i = 0; i < *nclasses; i++)
     {
         any_lib = (XIAnyClassInfo*)ptr_lib;
         any_wire = (xXIAnyInfo*)ptr_wire;
 
-        to->classes[i] = any_lib;
-        any_lib->type = any_wire->type;
-        any_lib->sourceid = any_wire->sourceid;
         switch(any_wire->type)
         {
             case XIButtonClass:
@@ -1477,37 +1538,52 @@ copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int nclasses)
                     xXIButtonInfo *cls_wire;
                     uint32_t *atoms;
                     int j;
+                    int struct_size;
+                    int state_size;
+                    int labels_size;
 
-                    cls_lib = next_block(&ptr_lib, sizeof(XIButtonClassInfo));
                     cls_wire = (xXIButtonInfo*)any_wire;
+                    sizeXIButtonClassType(cls_wire->num_buttons,
+                                          &struct_size, &state_size,
+                                          &labels_size);
+                    cls_lib = next_block(&ptr_lib, struct_size);
 
+                    cls_lib->type = cls_wire->type;
+                    cls_lib->sourceid = cls_wire->sourceid;
                     cls_lib->num_buttons = cls_wire->num_buttons;
-                    cls_lib->state.mask_len = ((((cls_wire->num_buttons + 7)/8) + 3)/4) * 4;
-                    cls_lib->state.mask = next_block(&ptr_lib, cls_lib->state.mask_len);
+                    cls_lib->state.mask_len = state_size;
+                    cls_lib->state.mask = next_block(&ptr_lib, state_size);
                     memcpy(cls_lib->state.mask, &cls_wire[1],
                            cls_lib->state.mask_len);
 
-                    cls_lib->labels = next_block(&ptr_lib, cls_lib->num_buttons * sizeof(Atom));
+                    cls_lib->labels = next_block(&ptr_lib, labels_size);
                     atoms =(uint32_t*)((char*)&cls_wire[1] + cls_lib->state.mask_len);
                     for (j = 0; j < cls_lib->num_buttons; j++)
                         cls_lib->labels[j] = *atoms++;
 
+                    to->classes[cls_idx++] = any_lib;
                     break;
                 }
             case XIKeyClass:
                 {
                     XIKeyClassInfo *cls_lib;
                     xXIKeyInfo *cls_wire;
+                    int struct_size;
+                    int keycodes_size;
 
-                    cls_lib = next_block(&ptr_lib, sizeof(XIKeyClassInfo));
                     cls_wire = (xXIKeyInfo*)any_wire;
+                    sizeXIKeyClassType(cls_wire->num_keycodes,
+                                       &struct_size, &keycodes_size);
+                    cls_lib = next_block(&ptr_lib, struct_size);
 
+                    cls_lib->type = cls_wire->type;
+                    cls_lib->sourceid = cls_wire->sourceid;
                     cls_lib->num_keycodes = cls_wire->num_keycodes;
-                    cls_lib->keycodes = next_block(&ptr_lib,
-                            cls_lib->num_keycodes * sizeof(int));
+                    cls_lib->keycodes = next_block(&ptr_lib, keycodes_size);
                     memcpy(cls_lib->keycodes, &cls_wire[1],
                             cls_lib->num_keycodes);
 
+                    to->classes[cls_idx++] = any_lib;
                     break;
                 }
             case XIValuatorClass:
@@ -1515,9 +1591,13 @@ copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int nclasses)
                     XIValuatorClassInfo *cls_lib;
                     xXIValuatorInfo *cls_wire;
 
-                    cls_lib = next_block(&ptr_lib, sizeof(XIValuatorClassInfo));
+                    cls_lib =
+                      next_block(&ptr_lib,
+                                 sizeDeviceClassType(XIValuatorClass, 0));
                     cls_wire = (xXIValuatorInfo*)any_wire;
 
+                    cls_lib->type = cls_wire->type;
+                    cls_lib->sourceid = cls_wire->sourceid;
                     cls_lib->number = cls_wire->number;
                     cls_lib->label  = cls_wire->label;
                     cls_lib->resolution = cls_wire->resolution;
@@ -1527,12 +1607,16 @@ copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int nclasses)
                     /* FIXME: fractional parts */
                     cls_lib->mode       = cls_wire->mode;
 
+                    to->classes[cls_idx++] = any_lib;
                 }
                 break;
         }
         len += any_wire->length * 4;
         ptr_wire += any_wire->length * 4;
     }
+
+    /* we may have skipped unknown classes, reset nclasses */
+    *nclasses = cls_idx;
     return len;
 }
 
@@ -1543,6 +1627,7 @@ wireToDeviceChangedEvent(xXIDeviceChangedEvent *in, XGenericEventCookie *cookie)
     XIDeviceChangedEvent *out;
     XIDeviceInfo info;
     int len;
+    int nclasses = in->num_classes;
 
     len = size_classes((xXIAnyInfo*)&in[1], in->num_classes);
 
@@ -1557,13 +1642,13 @@ wireToDeviceChangedEvent(xXIDeviceChangedEvent *in, XGenericEventCookie *cookie)
     out->deviceid = in->deviceid;
     out->sourceid = in->sourceid;
     out->reason = in->reason;
-    out->num_classes = in->num_classes;
 
     out->classes = (XIAnyClassInfo**)&out[1];
 
     info.classes = out->classes;
 
-    copy_classes(&info, (xXIAnyInfo*)&in[1], in->num_classes);
+    copy_classes(&info, (xXIAnyInfo*)&in[1], &nclasses);
+    out->num_classes = nclasses;
 
     return 1;
 }
diff --git a/src/XIGrabDevice.c b/src/XIGrabDevice.c
index 985d3f1..e0ca601 100644
--- a/src/XIGrabDevice.c
+++ b/src/XIGrabDevice.c
@@ -59,10 +59,10 @@ XIGrabDevice(Display* dpy, int deviceid, Window grab_window, Time time,
     req->cursor = cursor;
 
 
-    /* masks.mask_len is in bytes, but we need 4-byte units on the wire,
+    /* mask->mask_len is in bytes, but we need 4-byte units on the wire,
      * and they need to be padded with 0 */
-    len = req->mask_len * 4;
-    buff = calloc(1, len);
+    len = req->mask_len;
+    buff = calloc(1, len * 4);
     memcpy(buff, mask->mask, mask->mask_len);
 
     SetReqLen(req, len, len);
diff --git a/src/XIHierarchy.c b/src/XIHierarchy.c
index 9b267bf..20e5562 100644
--- a/src/XIHierarchy.c
+++ b/src/XIHierarchy.c
@@ -52,6 +52,9 @@ XIChangeHierarchy(Display* dpy,
     if (_XiCheckExtInit(dpy, Dont_Check, info) == -1)
 	return (NoSuchExtension);
 
+    if (num_changes <= 0)
+        return Success;
+
     GetReq(XIChangeHierarchy, req);
     req->reqType = info->codes->major_opcode;
     req->ReqType = X_XIChangeHierarchy;
diff --git a/src/XIPassiveGrab.c b/src/XIPassiveGrab.c
index 8953013..bdbe55a 100644
--- a/src/XIPassiveGrab.c
+++ b/src/XIPassiveGrab.c
@@ -67,13 +67,13 @@ _XIPassiveGrabDevice(Display* dpy, int deviceid, int grabtype, int detail,
 
     buff = calloc(4, req->mask_len);
     memcpy(buff, mask->mask, mask->mask_len);
-    Data32(dpy, buff, req->mask_len * 4);
+    Data(dpy, buff, req->mask_len * 4);
     for (i = 0; i < num_modifiers; i++)
-        Data32(dpy, &modifiers_inout[i].modifiers, 4);
+        Data(dpy, (char*)&modifiers_inout[i].modifiers, 4);
 
     free(buff);
 
-    if (_XReply(dpy, (xReply *)&reply, 0, xTrue))
+    if (!_XReply(dpy, (xReply *)&reply, 0, xFalse))
     {
 	UnlockDisplay(dpy);
 	SyncHandle();
diff --git a/src/XIQueryDevice.c b/src/XIQueryDevice.c
index 34c38cb..67319b7 100644
--- a/src/XIQueryDevice.c
+++ b/src/XIQueryDevice.c
@@ -29,7 +29,7 @@
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
 
-extern int copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int nclasses);
+extern int copy_classes(XIDeviceInfo* to, xXIAnyInfo* from, int *nclasses);
 extern int size_classes(xXIAnyInfo* from, int nclasses);
 
 XIDeviceInfo*
@@ -70,6 +70,8 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)
 
     for (i = 0; i < reply.num_devices; i++)
     {
+        int             nclasses;
+        size_t          sz;
         XIDeviceInfo    *lib = &info[i];
         xXIDeviceInfo   *wire = (xXIDeviceInfo*)ptr;
 
@@ -77,8 +79,7 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)
         lib->use         = wire->use;
         lib->attachment  = wire->attachment;
         lib->enabled     = wire->enabled;
-        lib->num_classes = wire->num_classes;
-        lib->classes     = (XIAnyClassInfo**)&lib[1];
+        nclasses         = wire->num_classes;
 
         ptr += sizeof(xXIDeviceInfo);
 
@@ -86,8 +87,11 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)
         strncpy(lib->name, ptr, wire->name_len);
         ptr += ((wire->name_len + 3)/4) * 4;
 
-        lib->classes = Xmalloc(size_classes((xXIAnyInfo*)ptr, lib->num_classes));
-        ptr += copy_classes(lib, (xXIAnyInfo*)ptr, lib->num_classes);
+        sz = size_classes((xXIAnyInfo*)ptr, nclasses);
+        lib->classes = Xmalloc(sz);
+        ptr += copy_classes(lib, (xXIAnyInfo*)ptr, &nclasses);
+        /* We skip over unused classes */
+        lib->num_classes = nclasses;
     }
 
     Xfree(buf);
diff --git a/src/XIQueryPointer.c b/src/XIQueryPointer.c
index b3bfebc..e068a97 100644
--- a/src/XIQueryPointer.c
+++ b/src/XIQueryPointer.c
@@ -86,9 +86,12 @@ XIQueryPointer(Display     *dpy,
     mods->base          = rep.mods.base_mods;
     mods->latched       = rep.mods.latched_mods;
     mods->locked        = rep.mods.locked_mods;
+    mods->effective     = mods->base | mods->latched | mods->locked;
+
     group->base         = rep.group.base_group;
     group->latched      = rep.group.latched_group;
     group->locked       = rep.group.locked_group;
+    group->effective    = group->base | group->latched | group->locked;
 
     buttons->mask_len   = rep.buttons_len * 4;
     buttons->mask       = malloc(buttons->mask_len);
diff --git a/src/XISelEv.c b/src/XISelEv.c
index dad890e..9a6fd2f 100644
--- a/src/XISelEv.c
+++ b/src/XISelEv.c
@@ -83,7 +83,7 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* masks, int num_masks)
          * and they need to be padded with 0 */
         buff = calloc(1, mask.mask_len * 4);
         memcpy(buff, current->mask, current->mask_len);
-        Data32(dpy, &mask, sizeof(xXIEventMask));
+        Data(dpy, (char*)&mask, sizeof(xXIEventMask));
         Data(dpy, buff, mask.mask_len * 4);
         free(buff);
     }
diff --git a/src/XListDev.c b/src/XListDev.c
index 46db220..458dd3d 100644
--- a/src/XListDev.c
+++ b/src/XListDev.c
@@ -60,6 +60,17 @@ SOFTWARE.
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
 
+/* Calculate length field to a multiples of sizeof(XID). XIDs are typedefs
+ * to ulong and thus may be 8 bytes on some platforms. This can trigger a
+ * SIGBUS if a class ends up not being 8-aligned (e.g. after XAxisInfo).
+ */
+static int pad_to_xid(int base_size)
+{
+    int padsize = sizeof(XID);
+
+    return ((base_size + padsize - 1)/padsize) * padsize;
+}
+
 static int
 SizeClassInfo(xAnyClassPtr *any, int num_classes)
 {
@@ -68,18 +79,18 @@ SizeClassInfo(xAnyClassPtr *any, int num_classes)
     for (j = 0; j < num_classes; j++) {
         switch ((*any)->class) {
             case KeyClass:
-                size += sizeof(XKeyInfo);
+                size += pad_to_xid(sizeof(XKeyInfo));
                 break;
             case ButtonClass:
-                size += sizeof(XButtonInfo);
+                size += pad_to_xid(sizeof(XButtonInfo));
                 break;
             case ValuatorClass:
                 {
                     xValuatorInfoPtr v;
 
                     v = (xValuatorInfoPtr) *any;
-                    size += sizeof(XValuatorInfo) +
-                        (v->num_axes * sizeof(XAxisInfo));
+                    size += pad_to_xid(sizeof(XValuatorInfo) +
+                        (v->num_axes * sizeof(XAxisInfo)));
                     break;
                 }
             default:
@@ -104,7 +115,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int num_classes)
                     xKeyInfoPtr k = (xKeyInfoPtr) *any;
 
                     K->class = KeyClass;
-                    K->length = sizeof(XKeyInfo);
+                    K->length = pad_to_xid(sizeof(XKeyInfo));
                     K->min_keycode = k->min_keycode;
                     K->max_keycode = k->max_keycode;
                     K->num_keys = k->num_keys;
@@ -116,7 +127,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int num_classes)
                     xButtonInfoPtr b = (xButtonInfoPtr) *any;
 
                     B->class = ButtonClass;
-                    B->length = sizeof(XButtonInfo);
+                    B->length = pad_to_xid(sizeof(XButtonInfo));
                     B->num_buttons = b->num_buttons;
                     break;
                 }
@@ -128,8 +139,8 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int num_classes)
                     xAxisInfoPtr a;
 
                     V->class = ValuatorClass;
-                    V->length = sizeof(XValuatorInfo) +
-                        (v->num_axes * sizeof(XAxisInfo));
+                    V->length = pad_to_xid(sizeof(XValuatorInfo) +
+                        (v->num_axes * sizeof(XAxisInfo)));
                     V->num_axes = v->num_axes;
                     V->motion_buffer = v->motion_buffer_size;
                     V->mode = v->mode;

Cheers,
Julien

Attachment: signature.asc
Description: Digital signature


Reply to: