Bug 27657

Summary: xinput crashes with bad atom on 64bit systems
Product: xorg Reporter: Kees Cook <kees>
Component: App/xinputAssignee: Peter Hutterer <peter.hutterer>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium Keywords: patch
Version: git   
Hardware: x86-64 (AMD64)   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
fix Atom width none

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

https://launchpad.net/bugs/563457

$ 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
ping?
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.