Bug 6472 - readKernelMapping overruns 'map' table.
Summary: readKernelMapping overruns 'map' table.
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Input/Keyboard (show other bugs)
Version: 7.0.0
Hardware: All Linux (All)
: high critical
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords:
: 6533 (view as bug list)
Depends on:
Blocks: 5387
  Show dependency treegraph
 
Reported: 2006-04-03 02:22 UTC by David Woodhouse
Modified: 2006-04-24 12:24 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Potential fix. (468 bytes, patch)
2006-04-03 02:39 UTC, David Woodhouse
no flags Details | Splinter Review

Description David Woodhouse 2006-04-03 02:22:30 UTC
When reading the kernel keyboard mapping (readKernelMapping in
os-support/linux/lnx_KbdMap.c) we overrun the usefully-named global array 'map',
scribbling on other random static variables elsewhere. 

The debugging patch attached to Red Hat bug #187083
(https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=127207) show this clearly...

readKernelMapping. map 0x10213488 is of size 0xf80 (i.e. ends 0x10214408)
NUM_CUSTOMKEYS 128, NUM_AT2LNX 248, NUM_KEYCODES 248, GLYPHS_PER_KEY 4
i is 0, j is 0, k is 0x10213498
i is 0, j is 1, k is 0x1021349c
i is 0, j is 2, k is 0x102134a0
i is 0, j is 3, k is 0x102134a4
i is 1, j is 0, k is 0x102134a8
 ....
i is 245, j is 3, k is 0x102143f4
i is 246, j is 0, k is 0x102143f8
i is 246, j is 1, k is 0x102143fc
i is 246, j is 2, k is 0x10214400
i is 246, j is 3, k is 0x10214404
i is 247, j is 0, k is 0x10214408
i is 247, j is 1, k is 0x1021440c
i is 247, j is 2, k is 0x10214410
i is 247, j is 3, k is 0x10214414

... with the last four writes scribbling on random variables elsewhere.

In the Fedora Core 5 build for PowerPC, those just happened to be the 'fd' in
linuxPciOpenFile(), so on VT switch away and back again, we closed and reopened
the wrong file descriptor... then started writing log text to the PCI config
space instead of the logfile, with predictably useful results.
Comment 1 David Woodhouse 2006-04-03 02:27:42 UTC
Oh, the 'map' array isn't actually global -- it's just declared static in the
header file. That's, erm, interesting :)
Comment 2 David Woodhouse 2006-04-03 02:39:20 UTC
Created attachment 5169 [details] [review]
Potential fix.
Comment 3 Adam Jackson 2006-04-15 05:10:21 UTC
kem claimed he had a better fix, iirc.
Comment 4 Michel Dänzer 2006-04-16 01:15:34 UTC
*** Bug 6533 has been marked as a duplicate of this bug. ***
Comment 5 David Woodhouse 2006-04-18 01:33:36 UTC
Is kem going to share his fix with us?
Comment 6 Matthias Dahl 2006-04-19 06:58:48 UTC
I spent weeks on the track of this bug with gdb and several other things. (bug 
6533) My symptons were a bit different: if Xorg was compiled with anything 
above -O0 and gcc >=4.1.x, starting with the second VT switch, the screen 
stayed black (or the system just hanged when switched back from the VT with the 
native nvidia driver). After I finally narrowed it down to readKernelMapping, 
this bug here popped up. :-)

To make a long story short: I consider this a major bug as memory is getting 
corrupted which naturally causes undesired and unforeseeable effects. Having an 
official fix for this would be rather important, so distributions could 
incorporate it into their packages until a new release is out.
Comment 7 Kevin E. Martin 2006-04-20 17:19:06 UTC
Just saw benh's question in irc -- didn't know that people were waiting on my fix.

My fix just rewrote a bit of the code to make it clearer what is going on here
and get rid of the map+GLYPHS_PER_KEY hack -- i.e., it's nothing special. 
Unfortunately, it was lost when my laptop hard drive failed earlier this week. 
It shouldn't be too hard to recreate, but I won't have time to do that until I
return from my trip next week.

For 7.1, I think David's fix is fine and should go in.
Comment 8 Benjamin Herrenschmidt 2006-04-22 13:23:01 UTC
I commited the fix to HEAD. I'll let Ajax decide if it should also go to some branch
Comment 9 Adam Jackson 2006-04-25 05:24:21 UTC
(In reply to comment #8)
> I commited the fix to HEAD. I'll let Ajax decide if it should also go to some
branch

It does; applied to 1.1 branch.  Probably should go in 1.0 branch too, if we
ever do another 1.0.x; moving to appropriate tracker.


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.