Skip to contentSkip to conversation

vnc: fix use-after-free

PATCH In review Gerd Hoffmann 4 participants 4 messages Apr 20, 2018

lore
Share & tools
lore

Gerd Hoffmann

author
#0 lore raw

PATCH 1/1 vnc: fix use-after-free

When vnc_client_read() return value is -1
vs is not valid any more.

Fixes d49b87f0d1e0520443a990fc610d0f02bc63c556
Reported-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>
---
 ui/vnc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
ui/vnc.c +3 -2
index e164eb798c..5526e54f48 100644
@@ -1539,13 +1539,14 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
     VncState *vs = opaque;
     if (condition & G_IO_IN) {
         if (vnc_client_read(vs) < 0) {
-            goto end;
+            /* vs is free()ed here */
+            return TRUE;
         }
     }
     if (condition & G_IO_OUT) {
         vnc_client_write(vs);
     }
-end:
+
     if (vs->disconnecting) {
         if (vs->ioc_tag != 0) {
             g_source_remove(vs->ioc_tag);
Signature · 1 line
2.9.3
Tested-by Reviewed-by on v1 1/1
#1 lore raw

Hi

Quoted 6 lines · Gerd Hoffmann

When vnc_client_read() return value is -1
vs is not valid any more.

Fixes d49b87f0d1e0520443a990fc610d0f02bc63c556
Reported-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>
Tested-by Marc-André Lureau <[email protected]>
Reviewed-by Marc-André Lureau <[email protected]>
Quoted 29 lines
---
 ui/vnc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
ui/vnc.c +3 -3
index e164eb798c..5526e54f48 100644
@@ -1539,13 +1539,14 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
     VncState *vs = opaque;
     if (condition & G_IO_IN) {
         if (vnc_client_read(vs) < 0) {
-            goto end;
+            /* vs is free()ed here */
+            return TRUE;
         }
     }
     if (condition & G_IO_OUT) {
         vnc_client_write(vs);
     }
-end:
+
     if (vs->disconnecting) {
         if (vs->ioc_tag != 0) {
             g_source_remove(vs->ioc_tag);
--

2.9.3

Signature · 1 line
Marc-André Lureau
Reviewed-by on v1 1/1
#2 lore raw
In reply to Gerd Hoffmann
Quoted 9 lines · Gerd Hoffmann

When vnc_client_read() return value is -1
vs is not valid any more.

Fixes d49b87f0d1e0520443a990fc610d0f02bc63c556
Reported-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>
---
 ui/vnc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by Daniel P. Berrangé <[email protected]>

Regards,
Daniel

Signature · 3 lines
|: 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 :|
Reviewed-by Tested-by on v1 1/1
#3 lore raw
In reply to Gerd Hoffmann

Cc'ing qemu-stable

Quoted 6 lines · Gerd Hoffmann

When vnc_client_read() return value is -1
vs is not valid any more.

Fixes d49b87f0d1e0520443a990fc610d0f02bc63c556
Reported-by Philippe Mathieu-Daudé <[email protected]>
Signed-off-by Gerd Hoffmann <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Tested-by Philippe Mathieu-Daudé <[email protected]>
Quoted 26 lines
---
 ui/vnc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
ui/vnc.c +3 -2
index e164eb798c..5526e54f48 100644
@@ -1539,13 +1539,14 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
     VncState *vs = opaque;
     if (condition & G_IO_IN) {
         if (vnc_client_read(vs) < 0) {
-            goto end;
+            /* vs is free()ed here */
+            return TRUE;
         }
     }
     if (condition & G_IO_OUT) {
         vnc_client_write(vs);
     }
-end:
+
     if (vs->disconnecting) {
         if (vs->ioc_tag != 0) {
             g_source_remove(vs->ioc_tag);