On one of our client's node, due to trying to read from closed ioc,
a segmentation fault occured. Corresponding backtrace:

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.

Signed-off-by: Klim Kireev <klim.kireev@virtuozzo.com>
---
 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));
-- 
2.14.3

Hi

On Wed, Jan 31, 2018 at 2:06 PM, Klim Kireev <klim.kireev@virtuozzo.com> wrote:
> On one of our client's node, due to trying to read from closed ioc,
> a segmentation fault occured. Corresponding backtrace:
>

Oops, you probably forgot an extra space before the # interpreted as comment.

Do you have a reproducer?

> 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.


>
> Signed-off-by: Klim Kireev <klim.kireev@virtuozzo.com>
> ---
>  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));
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

On 01/31/2018 04:16 PM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jan 31, 2018 at 2:06 PM, Klim Kireev <klim.kireev@virtuozzo.com> wrote:
>> On one of our client's node, due to trying to read from closed ioc,
>> a segmentation fault occured. Corresponding backtrace:
>>
> Oops, you probably forgot an extra space before the # interpreted as comment.
Thanks, fixed
>
> Do you have a reproducer?
Unfortunately, no
>> 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.
>
>> Signed-off-by: Klim Kireev <klim.kireev@virtuozzo.com>
>> ---
>>   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));
>> --
>> 2.14.3
>>
>>
>
>