From f390cef507bddd1cfa443c7e1eca03cc0f265041 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 8 Jun 2010 15:54:45 -0300
Subject: [PATCH 1/5] Fix changing password using monitor over VNC.

RH-Author: Gerd Hoffmann <kraxel@redhat.com>
Message-id: <1276012485-26618-1-git-send-email-kraxel@redhat.com>
Patchwork-id: 9784
O-Subject: [RHEL-5 kvm PATCH] Fix changing password using monitor over VNC.
Bugzilla: 569743
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
RH-Acked-by: Juan Quintela <quintela@redhat.com>
RH-Acked-by: Zachary Amsden <zamsden@redhat.com>

From: Zachary Amsden <zamsden at>

A simple segfault turned out to be a relatively complex fix.

The monitor calls back to main_loop_wait() to wait for the completion
of the password change event; this results in a nested invocation of
the associated I/O handlers.  For stdio monitor, this is okay, but VNC
maintains an input buffer which is not flushed until after the
invocation of protocol actions.  This is non-reentrant; the result is
that the nested invocation consumes the same protocol event as the
parent (which was a '\n', setting a NULL password), and it gets worse
when both the child and the parent attempt to shift in the same input
event, resulting in a memmove of size -1ULL, and a segfault.

The fix is to consume the input buffer before invoking protocol actions
which may cause nested invocation of the handler; we must also set up
the child handler to receive new events, which was cleanest done with
vnc_read_when() from the protcol handler (doing it in the outer loop
causes bugs with other types of waits, such as auth).  We return fed=1
from the outer handler to prevent the logic in vnc_client_read from
reconsuming the pre-consumed buffer, and simply reset the expect
value to receive the next protocol command.

Signed-off-by: Zachary Amsden <zamsden@redhat.com>

Upstream fixed this in a different way:  Rework monitor to not call back
into main_loop_wait in the first place.  But backporting these patches
is to invasive for RHEL-5 IMHO.  Additionally they also change qemu
behavior in case password-protected blockdevs are used.

bugzilla: #569743
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu/vnc.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu/vnc.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/qemu/vnc.c b/qemu/vnc.c
index 08db1db..b01cb01 100644
--- a/qemu/vnc.c
+++ b/qemu/vnc.c
@@ -900,6 +900,15 @@ static void vnc_read_when(VncState *vs, VncReadEvent *func, size_t expecting)
     vs->read_handler_expect = expecting;
 }
 
+static void vnc_client_consume(VncState *vs, size_t len)
+{
+    if (vs->input.offset >= len) {
+        memmove(vs->input.buffer, vs->input.buffer + len,
+                (vs->input.offset - len));
+        vs->input.offset -= len;
+    }
+}
+
 static void vnc_client_read(void *opaque)
 {
     VncState *vs = opaque;
@@ -940,8 +949,7 @@ static void vnc_client_read(void *opaque)
         }
 
 	if (!ret) {
-	    memmove(vs->input.buffer, vs->input.buffer + len, (vs->input.offset - len));
-	    vs->input.offset -= len;
+            vnc_client_consume(vs, len);
 	} else {
 	    vs->read_handler_expect = ret;
 	}
@@ -1655,6 +1663,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 {
     int i;
     uint16_t limit;
+    int fed = 0;
 
     switch (data[0]) {
     case 0:
@@ -1697,6 +1706,9 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 	if (len == 1)
 	    return 8;
 
+        vnc_client_consume(vs, len);
+        vnc_read_when(vs, protocol_client_msg, 1);
+        fed = 1;
 	key_event(vs, read_u8(data, 1), read_u32(data, 4));
 	break;
     case 5:
@@ -1726,6 +1738,9 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
             if (len == 2)
                 return 12;
 
+            vnc_client_consume(vs, len);
+            vnc_read_when(vs, protocol_client_msg, 1);
+            fed = 1;
             ext_key_event(vs, read_u16(data, 2),
                           read_u32(data, 4), read_u32(data, 8));
             break;
@@ -1784,7 +1799,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
     }
 
     vnc_read_when(vs, protocol_client_msg, 1);
-    return 0;
+    return fed;
 }
 
 static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
-- 
1.7.0.3