[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] vnc: fix segfault in closed connection handl
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH v2] vnc: fix segfault in closed connection handling |
Date: |
Wed, 31 Jan 2018 13:22:35 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Jan 31, 2018 at 04:15:37PM +0300, Klim Kireev wrote:
> On one of our client's node, due to trying to read from closed ioc,
> a segmentation fault occured. Corresponding backtrace:
>
> 0 object_get_class (address@hidden)
> 1 qio_channel_readv_full (ioc=0x0, iov=0x7ffe55277180 ...
> 2 qio_channel_read (ioc=<optimized out> ...
> 3 vnc_client_read_buf (address@hidden, ...
> 4 vnc_client_read_plain (vs=0x55625f3c6000)
> 5 vnc_client_read (vs=0x55625f3c6000)
> 6 vnc_client_io (ioc=<optimized out>, condition=G_IO_IN, ...
> 7 g_main_dispatch (context=0x556251568a50)
> 8 g_main_context_dispatch (address@hidden)
> 9 glib_pollfds_poll ()
> 10 os_host_main_loop_wait (timeout=<optimized out>)
> 11 main_loop_wait (address@hidden)
> 12 main_loop () at vl.c:1909
> 13 main (argc=<optimized out>, argv=<optimized out>, ...
>
> Having analyzed the coredump, I understood that the reason is that
> ioc_tag is reset on vnc_disconnect_start and ioc is cleaned
> in vnc_disconnect_finish. Between these two events due to some
> reasons the ioc_tag was set again and after vnc_disconnect_finish
> the handler is running with freed ioc,
> which led to the segmentation fault.
>
> I suggest to check ioc_tag in vnc_disconnect_finish to prevent such
> an occurrence.
I think the root cause here is some codepath failing to check the
'vs->disconnecting' flag. eg vnc_write() checks that before it
calls qio_channel_add_watch. I suspect somewhere else in code needs
such a check
>
> Signed-off-by: Klim Kireev <address@hidden>
> ---
> Changelog:
> v2: Apply the backtrace
>
> ui/vnc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 33b087221f..b8bf0180cb 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1270,6 +1270,10 @@ void vnc_disconnect_finish(VncState *vs)
> }
> g_free(vs->lossy_rect);
>
> + if (vs->ioc_tag) {
> + g_source_remove(vs->ioc_tag);
> + vs->ioc_tag = 0;
> + }
> object_unref(OBJECT(vs->ioc));
> vs->ioc = NULL;
> object_unref(OBJECT(vs->sioc));
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|