ui: avoid crash if vnc client disconnects with writes pending

The vnc_client_read() function is called from the vnc_client_io()
event handler callback when there is incoming data to process.
If it detects that the client has disconnected, then it will
trigger cleanup and free'ing of the VncState client struct at
a safe time.

Unfortunately, the vnc_client_io() event handler will also call
vnc_client_write() to handle any outgoing data writes. So if
vnc_client_io() was invoked with both G_IO_IN and G_IO_OUT
events set, and the client disconnects, we may try to write to
a client which has just been freed.

https://bugs.launchpad.net/qemu/+bug/1594861

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1467042529-3372-1-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
(cherry picked from commit ea697449884d83b83fefbc9cd87bdde0c94b49d6)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

diff --git a/ui/vnc.c b/ui/vnc.c
index bca352e523d851cc534a74d17af9278c547e7674..55747cc830bd24dff6a671a824a5150be6ed12d6 100644 (file)
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1442,8 +1442,9 @@ static void vnc_jobs_bh(void *opaque)
  * First function called whenever there is more data to be read from
  * the client socket. Will delegate actual work according to whether
  * SASL SSF layers are enabled (thus requiring decryption calls)
+ * Returns 0 on success, -1 if client disconnected
  */
-static void vnc_client_read(VncState *vs)
+static int vnc_client_read(VncState *vs)
 {
     ssize_t ret;
 
@@ -1456,8 +1457,9 @@ static void vnc_client_read(VncState *vs)
     if (!ret) {
         if (vs->disconnecting) {
             vnc_disconnect_finish(vs);
+            return -1;
         }
-        return;
+        return 0;
     }
 
     while (vs->read_handler && vs->input.offset >= vs->read_handler_expect) {
@@ -1467,7 +1469,7 @@ static void vnc_client_read(VncState *vs)
         ret = vs->read_handler(vs, vs->input.buffer, len);
         if (vs->disconnecting) {
             vnc_disconnect_finish(vs);
-            return;
+            return -1;
         }
 
         if (!ret) {
@@ -1476,6 +1478,7 @@ static void vnc_client_read(VncState *vs)
             vs->read_handler_expect = ret;
         }
     }
+    return 0;
 }
 
 gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
@@ -1483,7 +1486,9 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
 {
     VncState *vs = opaque;
     if (condition & G_IO_IN) {
-        vnc_client_read(vs);
+        if (vnc_client_read(vs) < 0) {
+            return TRUE;
+        }
     }
     if (condition & G_IO_OUT) {
         vnc_client_write(vs);