Bug 27657 - xinput crashes with bad atom on 64bit systems
Summary: xinput crashes with bad atom on 64bit systems
Alias: None
Product: xorg
Classification: Unclassified
Component: App/xinput (show other bugs)
Version: git
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: Peter Hutterer
QA Contact: Xorg Project Team
Keywords: patch
Depends on:
Reported: 2010-04-14 21:26 UTC by Kees Cook
Modified: 2011-10-15 16:35 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

fix Atom width (1.81 KB, patch)
2010-04-14 21:26 UTC, Kees Cook
no flags Details | Splinter Review

Description Kees Cook 2010-04-14 21:26:44 UTC
Created attachment 35047 [details] [review]
fix Atom width


$ xinput list-props 14
 Wacom Button Actions (282): "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), "None" (0), X Error of failed request: BadAtom (invalid Atom parameter)
  Major opcode of failed request: 17 (X_GetAtomName)
  Atom id in failed request: 0x0
  Serial number of failed request: 57
  Current serial number in output stream: 57

Atoms are packed as 32bit values, and on 64bit systems, this will mean the last atom in the list copies 4 bytes out of xinput's heap after the end of the data buffer. This patch corrects the dereferenced size so that the resulting atom is zero-extended instead of filling the high half with garbage.
Comment 1 Kees Cook 2010-04-14 21:29:40 UTC
note that since the xorg/app/xinput git tree doesn't have it's own bugzilla category, I used Apps/other here.
Comment 2 Peter Hutterer 2010-04-15 17:41:08 UTC
i'm pretty sure the first hunk isn't needed since Xlib will return long arrays for 32-bit values (even on 64-bit). The conversion is done internally, hence the code should work as-is. the xi2 code looks good though.

you can easily test the above if you change the inputproto.pc file on your machine to announce version 1.5. then run configure again and it'll build without the xi2 bits - using xi1's property requests instead.
Comment 3 Peter Hutterer 2010-05-16 22:11:16 UTC
Comment 4 Kees Cook 2010-05-17 08:38:12 UTC
I haven't had a chance to test this, but since the xi2 stuff is correct, can we commit it?
Comment 5 Peter Hutterer 2010-05-17 17:34:26 UTC
Thanks for the patch, committed (in  half)

commit 87ec8d42c7f8e4e0613bcbe59fb2db991e1e4acb
Author: Kees Cook <kees.cook@canonical.com>
Date:   Wed Apr 14 21:19:48 2010 -0700

    Atoms from XIGetProperty are 32bits (#27657)

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.