[PATCH] dix: In MakeWindowOptional: Fix crash if parentOptional->cursor is NULL.

[Kim Woelders]

On Mon, 28 Sep 2009 01:28:24 +0200, Peter Hutterer  
<peter.hutterer at who-t.net> wrote:

> On Sun, Sep 27, 2009 at 09:16:00PM +0200, Kim Woelders wrote:
>> On Fri, 25 Sep 2009 03:11:59 +0200, Peter Hutterer
>> <peter.hutterer at who-t.net> wrote:
>>
>>> On Fri, Sep 25, 2009 at 12:05:35AM +0200, Kim Woelders wrote:
>>>>
>>>> Signed-off-by: Kim Woelders <kim at woelders.dk>
>>>> ---
>>>>  dix/window.c |    3 ++-
>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/dix/window.c b/dix/window.c
>>>> index caff1cb..d4bc41c 100644
>>>> --- a/dix/window.c
>>>> +++ b/dix/window.c
>>>> @@ -3470,7 +3470,8 @@ MakeWindowOptional (WindowPtr pWin)
>>>>      if (!pWin->cursorIsNone)
>>>>      {
>>>>  	optional->cursor = parentOptional->cursor;
>>>> -	optional->cursor->refcnt++;
>>>> +	if (optional->cursor)
>>>> +	    optional->cursor->refcnt++;
>>>>      }
>>>>      else
>>>>      {
>>>> --
>>>> 1.6.4.4
>>> Out of interest, what's the test scenario for that?
>>>
>>> I've looked at the code and can't find the trigger. If
>>> (!pWin->cursorIsNone),
>>> the window has to have a cursor set (and thus already have a
>>> WindowOptional).
>>> If the parent had a cursor and it was inherited here, then setting the
>>> parent to a NoneCursor should patch up the child accordingly anyway.
>>> So right now, I'm not sure about the actual flow to trigger that and
>>> worry
>>> that it may be part of a larger screwup.
>>>
>> Yeah, I was suspecting that this might not be a proper fix, and I must
>> admit I cannot reproduce the crash.
>
> Ok, I'll skip this patch then.
>
>> There definitely is something wrong though, I have the server crashing  
>> on
>> me in about ten different ways, and I'm suspecting some kind of memory
>> corruption.
>>
>> One crash I can reproduce reliably is this:
>>
>> Start server. Start xterm. Kill xterm. Server dies:
>
> try testing with -noreset, as Julien said the regeneration support was
> busted in the driver. If you see crashes that happen outside of a regen,
> that'd be serious.
>
Thanks, I updated to the intel driver from git and this problem seems to  
be gone.

Next reliable crash, although a bit harder to trigger, is this:

Start server. Start xterm. Start gqview and kill it "immediately", repeat  
start/kill gqview until server crash:

Program received signal SIGPIPE, Broken pipe.
0x007e3416 in __kernel_vsyscall ()
Missing separate debuginfos, use: debuginfo-install expat-2.0.1-7.i686  
freetype-2.3.9-6.fc12.i686 keyutils-libs-1.2-6.fc12.i686  
krb5-libs-1.7-8.fc12.i686 libattr-2.4.43-4.fc12.i686  
libcap-2.16-5.fc12.i686 libcom_err-1.41.9-3.fc12.i686  
libfontenc-1.0.5-2.fc12.i686 libgcc-4.4.1-17.i686  
mesa-dri-drivers-7.6-0.13.fc12.i686  
xorg-x11-drv-evdev-2.2.99-8.20090923.fc12.i686  
xorg-x11-drv-fbdev-0.4.1-1.fc12.i686  
xorg-x11-drv-synaptics-1.1.99-7.20090907.fc12.i686  
xorg-x11-drv-vesa-2.2.1-1.fc12.i686
(gdb) bt
#0  0x007e3416 in __kernel_vsyscall ()
#1  0x009eaecb in __libc_writev (fd=11067380, vector=0xbffbcf78, count=1)
     at ../sysdeps/unix/sysv/linux/writev.c:51
#2  0x080aa19f in _XSERVTransSocketWritev (ciptr=<value optimized out>,
     buf=<value optimized out>, size=<value optimized out>)
     at /usr/include/X11/Xtrans/Xtranssock.c:2184
#3  0x080a9940 in _XSERVTransWritev (ciptr=<value optimized out>,
     buf=<value optimized out>, size=<value optimized out>)
     at /usr/include/X11/Xtrans/Xtrans.c:912
#4  0x080a526e in FlushClient (who=<value optimized out>, oc=<value  
optimized out>,
     __extraBuf=<value optimized out>, extraCount=<value optimized out>) at  
io.c:898
#5  0x080a3b34 in CloseDownConnection (client=<value optimized out>)
     at connection.c:1063
#6  0x080691c5 in CloseDownClient (client=<value optimized out>) at  
dispatch.c:3608
#7  0x0806dff8 in Dispatch () at dispatch.c:456
#8  0x08062855 in main (argc=<value optimized out>, argv=<value optimized  
out>,
     envp=<value optimized out>) at main.c:285
(gdb)

It can be triggered with other apps than gqview but with gqview it is  
fairly easy.

Any suggestions?

/Kim