On Sun, 2009-05-24 at 22:08 +0200, Stefan Weil wrote: > Hello, > > this scenario crashs the latest QEMU HEAD on Windows > (Linux users, please note that the bug is not Windows related, > so don't stop reading!): > > * run qemu.exe -vnc :0 > * connect using UltraVnc > * select fuzzy screen mode in UltraVnc > > => segfault of qemu.exe > > The crash is caused by VNC protocols which are unsupported > by QEMU - in my case it was the fuzzy screen mode protocol. > These protocols trigger a call stack which releases the > VncState vs: > > qemu_free(vs) > vnc_client_io_error(vs, ...) > vnc_client_error(vs, ...) > protocol_client_msg(vs, ...) > vnc_client_read > main_loop_wait > main_loop > > The default handlers for unimplemented protocols in > protocol_client_msg call vnc_client_error which finally > calls qemu_free for the current VncState vs. > > vs is then used in protocol_client_msg and vnc_client_read > although it is no longer valid. On Windows, this results > in a crash, for other host platforms, the result depends > on implementation details of the C library. > > In any case, access to a data structure after a free() > is a bug. Yep, I looked into a similar report recently: https://bugzilla.redhat.com/show_bug.cgi?id=501131 Basically, vnc_flush() or vnc_update_client() will handle an error by freeing VncState, yet we will not detect this error and happily continue on using the freed VncState. The approach I tried in the patch attached to the bug isn't going to work, I think. Instead, we should just mark the VncState with a "deleted" flag on I/O error and only free it later on when the in-progress protocol step has completed. Cheers, Mark.