From patchwork Tue Jul 12 07:23:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 9224707 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DB1F3604DB for ; Tue, 12 Jul 2016 07:24:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C9CBB27165 for ; Tue, 12 Jul 2016 07:24:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BE26427F7F; Tue, 12 Jul 2016 07:24:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4EFD627165 for ; Tue, 12 Jul 2016 07:24:21 +0000 (UTC) Received: from localhost ([::1]:38038 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMs3I-00007Y-60 for patchwork-qemu-devel@patchwork.kernel.org; Tue, 12 Jul 2016 03:24:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMs2r-0008WR-47 for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:23:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bMs2l-0001Rf-Ah for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:23:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48510) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bMs2l-0001RK-4r for qemu-devel@nongnu.org; Tue, 12 Jul 2016 03:23:47 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5F1A060A4 for ; Tue, 12 Jul 2016 07:23:46 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-18.ams2.redhat.com [10.36.116.18]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6C7NjP3022619; Tue, 12 Jul 2016 03:23:45 -0400 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id CDCBA80F76; Tue, 12 Jul 2016 09:23:44 +0200 (CEST) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Tue, 12 Jul 2016 09:23:43 +0200 Message-Id: <1468308223-2496-4-git-send-email-kraxel@redhat.com> In-Reply-To: <1468308223-2496-1-git-send-email-kraxel@redhat.com> References: <1468308223-2496-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 12 Jul 2016 07:23:46 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 3/3] ui: avoid crash if vnc client disconnects with writes pending X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gerd Hoffmann Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: "Daniel P. Berrange" 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 Message-id: 1467042529-3372-1-git-send-email-berrange@redhat.com Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index bd602b6..e3f857c 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -1436,8 +1436,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; @@ -1450,8 +1451,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) { @@ -1461,7 +1463,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) { @@ -1470,6 +1472,7 @@ static void vnc_client_read(VncState *vs) vs->read_handler_expect = ret; } } + return 0; } gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED, @@ -1477,7 +1480,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);