Bug 19600

Created attachment 22020 [details] [review]
NULL pointer checks for exevents.c

Forwarding this bug from Ubuntu:
https://bugs.edge.launchpad.net/ubuntu/+source/xorg-server/+bug/311254

[Problem]
CopyKeyClass lacks a NULL pointer check for some parameters (notably mk) which apparently can be NULL in some circumstances.

[Discussion]
A crash occurs in CopyKeyClass with the following backtrace after pressing a button on an ATI USB remote control to seek backward in a video in totem.

At first we thought this might be a dupe of freedesktop-bugs #19222, however in that case master=0x0 whereas here master is fine; it's mk which is 0x0.

The crash can be reproduced reliably.  It did not happen in Ubuntu Intrepid (xserver 1.5.2)

#0 0x000000000053d155 in CopyKeyClass (device=0x1b55fa0, master=0x1ac2090)
    at /usr/include/bits/string3.h:52
        mk = (KeyClassPtr) 0x0
        dk = (KeyClassPtr) 0x1b566c0
        lastMapNotifyDevice = (DeviceIntPtr) 0x1ac2740
#1 0x00000000004cfa93 in mieqProcessInputEvents () at ../../mi/mieq.c:361
        handler = (mieqHandler) 0
        e = <value optimized out>
        type = <value optimized out>
        nevents = 1
        i = <value optimized out>
        screen = (ScreenPtr) 0x1937830
        event = (xEvent *) 0x2944fe0
        master_event = (xEvent *) 0x0
        dev = (DeviceIntPtr) 0x1b55fa0
        master = (DeviceIntPtr) 0x1ac2090
#2 0x0000000000484669 in ProcessInputEvents ()
    at ../../../../hw/xfree86/common/xf86Events.c:174
        x = 0
        y = 8232584
#3 0x000000000044d591 in Dispatch () at ../../dix/dispatch.c:363
        result = 0
        client = (ClientPtr) 0x1b337d0
        nready = -1
        start_tick = <value optimized out>
#4 0x00000000004332bd in main (argc=10, argv=0x7fff0f1bde78,
    envp=<value optimized out>) at ../../dix/main.c:383
        i = 1
        alwaysCheckForInput = {0, 1}

The attached patch adds several NULL pointer checks to make sure members of data structures aren't called if the data structure pointer is not valid.

However, I am left wondering why mk was NULL in the first place, and if this patch merely papers over a deeper issue...?