qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Fix changing password using monitor over VNC.
@ 2009-04-17  5:06 Zachary Amsden
  2009-04-17  7:00 ` [Qemu-devel] " Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Zachary Amsden @ 2009-04-17  5:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zachary Amsden

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>
---
 vnc.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/vnc.c b/vnc.c
index 2b3a6eb..3e88855 100644
--- a/vnc.c
+++ b/vnc.c
@@ -798,6 +798,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;
@@ -833,8 +842,7 @@ static void vnc_client_read(void *opaque)
 	    return;
 
 	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;
 	}
@@ -1357,6 +1365,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:
@@ -1399,6 +1408,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:
@@ -1428,6 +1440,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;
@@ -1486,7 +1501,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)
@@ -2285,6 +2300,8 @@ int vnc_display_password(DisplayState *ds, const char *password)
 	if (!(vs->password = qemu_strdup(password)))
 	    return -1;
     }
+    if (vs->auth == VNC_AUTH_NONE)
+        vs->auth = VNC_AUTH_VNC;
 
     return 0;
 }
-- 
1.6.2.2.471.g6da14

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-04-18  8:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-17  5:06 [Qemu-devel] [PATCH] Fix changing password using monitor over VNC Zachary Amsden
2009-04-17  7:00 ` [Qemu-devel] " Jan Kiszka
2009-04-17  7:43   ` Zachary Amsden
2009-04-17  8:31     ` Jan Kiszka
2009-04-18  3:31       ` Zachary Amsden
2009-04-18  8:41         ` Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).