From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LugIc-00074I-LH for qemu-devel@nongnu.org; Fri, 17 Apr 2009 01:07:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LugIV-000746-UT for qemu-devel@nongnu.org; Fri, 17 Apr 2009 01:07:37 -0400 Received: from [199.232.76.173] (port=48641 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LugIV-000743-QM for qemu-devel@nongnu.org; Fri, 17 Apr 2009 01:07:31 -0400 Received: from mx2.redhat.com ([66.187.237.31]:53765) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LugIV-00086N-6o for qemu-devel@nongnu.org; Fri, 17 Apr 2009 01:07:31 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3H57TWb016110 for ; Fri, 17 Apr 2009 01:07:29 -0400 From: Zachary Amsden Date: Thu, 16 Apr 2009 19:06:49 -1000 Message-Id: <1239944809-14327-1-git-send-email-zamsden@redhat.com> Subject: [Qemu-devel] [PATCH] Fix changing password using monitor over VNC. Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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 --- 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