Re: [Qemu-devel] vnc: heap-use-after-free in vnc_client_io() after vnc_disconnect_finish()

[Philippe Mathieu-Daudé]

On 04/19/2018 01:40 PM, Daniel P. Berrangé wrote:
> On Thu, Apr 19, 2018 at 06:37:12PM +0200, Marc-André Lureau wrote:
>> On Tue, Apr 17, 2018 at 7:52 PM, Philippe Mathieu-Daudé <address@hidden> 
>> wrote:
>>> running commit ce8d4082054519f2eaac39958edde502860a7fc6:
>>>
>>> qemu-system-mips -M malta -m 512 \
>>>   -kernel vmlinux-3.2.0-4-4kc-malta \
>>>   -append 'root=/dev/sda1 console=ttyS0' \
>>>   -hda debian_wheezy_mips_standard.qcow2 \
>>>   -redir tcp:5556::22 -serial stdio
>>>
>>> and connecting with "xtightvncviewer :0" I get when closing vnc:
>>
>> This is also true with other targets, ex:
>> x86_64-softmmu/qemu-system-x86_64 -vnc :0
>>
>> Daniel, Gerd, are you looking at it? this looks like a 2.12 regression.
> 
> I've not had toime to look at it - would be useful if someone can bisect
> it at least if its a regression since 2.11

My worst bisect so far, anyway here we go:

13e1d0e71e78a925848258391a6e616b6b5ae219 is the first bad commit
commit 13e1d0e71e78a925848258391a6e616b6b5ae219
Author: Daniel P. Berrange <address@hidden>
Date:   Thu Feb 1 16:45:14 2018 +0000

    ui: convert VNC server to QIONetListener

    The VNC server already has the ability to listen on multiple sockets.
    Converting it to use the QIONetListener APIs though, will reduce the
    amount of code in the VNC server and improve the clarity of what is
    left.

    Signed-off-by: Daniel P. Berrange <address@hidden>
    Message-id: address@hidden
    Signed-off-by: Gerd Hoffmann <address@hidden>


>>> ==27686==ERROR: AddressSanitizer: heap-use-after-free on address
>>> 0x63100003c814 at pc 0x55eed918362a bp 0x7ffdf5c36c80 sp 0x7ffdf5c36c78
>>> READ of size 4 at 0x63100003c814 thread T0
>>>     #0 0x55eed9183629 in vnc_client_io /source/qemu/ui/vnc.c:1549
>>>     #1 0x55eed94ae26c in qio_channel_fd_source_dispatch
>>> /source/qemu/io/channel-watch.c:84
>>>     #2 0x7f3e181860f4 in g_main_context_dispatch
>>> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c0f4)
>>>     #3 0x55eed95b9799 in glib_pollfds_poll /source/qemu/util/main-loop.c:215
>>>     #4 0x55eed95b9989 in os_host_main_loop_wait
>>> /source/qemu/util/main-loop.c:263
>>>     #5 0x55eed95b9b5f in main_loop_wait /source/qemu/util/main-loop.c:522
>>>     #6 0x55eed898f7dd in main_loop /source/qemu/vl.c:1943
>>>     #7 0x55eed89a1f0b in main /source/qemu/vl.c:4734
>>>     #8 0x7f3dfe094a86 in __libc_start_main
>>> (/lib/x86_64-linux-gnu/libc.so.6+0x21a86)
>>>     #9 0x55eed8518db9 in _start
>>> (/build/qemu/mips-softmmu/qemu-system-mips+0x14f3db9)
>>>
>>> 0x63100003c814 is located 20 bytes inside of 75744-byte region
>>> [0x63100003c800,0x63100004efe0)
>>> freed by thread T0 here:
>>>     #0 0x7f3e18b878c8 in __interceptor_free
>>> (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd98c8)
>>>     #1 0x55eed9181bb0 in vnc_disconnect_finish /source/qemu/ui/vnc.c:1278
>>>     #2 0x55eed9183225 in vnc_client_read /source/qemu/ui/vnc.c:1511
>>>     #3 0x55eed91835a9 in vnc_client_io /source/qemu/ui/vnc.c:1541
>>>     #4 0x55eed94ae26c in qio_channel_fd_source_dispatch
>>> /source/qemu/io/channel-watch.c:84
>>>     #5 0x7f3e181860f4 in g_main_context_dispatch
>>> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c0f4)
>>>
>>> previously allocated by thread T0 here:
>>>     #0 0x7f3e18b87df8 in calloc
>>> (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9df8)
>>>     #1 0x7f3e1818b8b0 in g_malloc0
>>> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x518b0)
>>>     #2 0x55eed9192aa0 in vnc_listen_io /source/qemu/ui/vnc.c:3186
>>>     #3 0x55eed94b9fd7 in qio_net_listener_channel_func
>>> /source/qemu/io/net-listener.c:57
>>>     #4 0x55eed94ae26c in qio_channel_fd_source_dispatch
>>> /source/qemu/io/channel-watch.c:84
>>>     #5 0x7f3e181860f4 in g_main_context_dispatch
>>> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c0f4)
>>>
>>> SUMMARY: AddressSanitizer: heap-use-after-free
>>> /source/qemu/ui/vnc.c:1549 in vnc_client_io
>>> Shadow bytes around the buggy address:
>>>   0x0c627ffff8b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>>   0x0c627ffff8c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>>   0x0c627ffff8d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>>   0x0c627ffff8e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>>   0x0c627ffff8f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>> =>0x0c627ffff900: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>   0x0c627ffff910: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>   0x0c627ffff920: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>   0x0c627ffff930: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>   0x0c627ffff940: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>   0x0c627ffff950: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>>>
>>> Shadow byte legend (one shadow byte represents 8 application bytes):
>>>   Heap left redzone:       fa
>>>   Freed heap region:       fd
>>> ==26606==ABORTING