From patchwork Mon Jun 6 07:06:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 9157297 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 4F29560759 for ; Mon, 6 Jun 2016 07:08:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 40A5D26907 for ; Mon, 6 Jun 2016 07:08:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 358FE27B13; Mon, 6 Jun 2016 07:08:05 +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=-5.9 required=2.0 tests=BAYES_00,FSL_HELO_HOME, 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 C5991281BE for ; Mon, 6 Jun 2016 07:08:04 +0000 (UTC) Received: from localhost ([::1]:40299 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9odn-0002Jz-Oy for patchwork-qemu-devel@patchwork.kernel.org; Mon, 06 Jun 2016 03:08:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9ocL-0001PH-5P for qemu-devel@nongnu.org; Mon, 06 Jun 2016 03:06:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9ocF-0006cJ-11 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 03:06:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53994) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9ocE-0006c8-OZ; Mon, 06 Jun 2016 03:06:26 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 475C21555C; Mon, 6 Jun 2016 07:06:26 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-43.ams2.redhat.com [10.36.116.43]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5676Pwr027322; Mon, 6 Jun 2016 03:06:25 -0400 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id B8F9A8276A; Mon, 6 Jun 2016 09:06:24 +0200 (CEST) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Mon, 6 Jun 2016 09:06:20 +0200 Message-Id: <1465196782-28073-5-git-send-email-kraxel@redhat.com> In-Reply-To: <1465196782-28073-1-git-send-email-kraxel@redhat.com> References: <1465196782-28073-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 06 Jun 2016 07:06:26 +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 4/6] vmsvga: shadow fifo registers 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: P J P , Gerd Hoffmann , qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP The fifo is normal ram. So kvm vcpu threads and qemu iothread can access the fifo in parallel without syncronization. Which in turn implies we can't use the fifo pointers in-place because the guest can try changing them underneath us. So add shadows for them, to make sure the guest can't modify them after we've applied sanity checks. Fixes: CVE-2016-4454 Cc: qemu-stable@nongnu.org Cc: P J P Signed-off-by: Gerd Hoffmann Message-id: 1464592161-18348-4-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 57 ++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index a26e62e..de2567b 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -66,17 +66,11 @@ struct vmsvga_state_s { uint8_t *fifo_ptr; unsigned int fifo_size; - union { - uint32_t *fifo; - struct QEMU_PACKED { - uint32_t min; - uint32_t max; - uint32_t next_cmd; - uint32_t stop; - /* Add registers here when adding capabilities. */ - uint32_t fifo[0]; - } *cmd; - }; + uint32_t *fifo; + uint32_t fifo_min; + uint32_t fifo_max; + uint32_t fifo_next; + uint32_t fifo_stop; #define REDRAW_FIFO_LEN 512 struct vmsvga_rect_s { @@ -198,7 +192,7 @@ enum { */ SVGA_FIFO_MIN = 0, SVGA_FIFO_MAX, /* The distance from MIN to MAX must be at least 10K */ - SVGA_FIFO_NEXT_CMD, + SVGA_FIFO_NEXT, SVGA_FIFO_STOP, /* @@ -546,8 +540,6 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s, } #endif -#define CMD(f) le32_to_cpu(s->cmd->f) - static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) { int num; @@ -556,38 +548,44 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) return 0; } + s->fifo_min = le32_to_cpu(s->fifo[SVGA_FIFO_MIN]); + s->fifo_max = le32_to_cpu(s->fifo[SVGA_FIFO_MAX]); + s->fifo_next = le32_to_cpu(s->fifo[SVGA_FIFO_NEXT]); + s->fifo_stop = le32_to_cpu(s->fifo[SVGA_FIFO_STOP]); + /* Check range and alignment. */ - if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { + if ((s->fifo_min | s->fifo_max | s->fifo_next | s->fifo_stop) & 3) { return 0; } - if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { + if (s->fifo_min < sizeof(uint32_t) * 4) { return 0; } - if (CMD(max) > SVGA_FIFO_SIZE || - CMD(min) >= SVGA_FIFO_SIZE || - CMD(stop) >= SVGA_FIFO_SIZE || - CMD(next_cmd) >= SVGA_FIFO_SIZE) { + if (s->fifo_max > SVGA_FIFO_SIZE || + s->fifo_min >= SVGA_FIFO_SIZE || + s->fifo_stop >= SVGA_FIFO_SIZE || + s->fifo_next >= SVGA_FIFO_SIZE) { return 0; } - if (CMD(max) < CMD(min) + 10 * 1024) { + if (s->fifo_max < s->fifo_min + 10 * 1024) { return 0; } - num = CMD(next_cmd) - CMD(stop); + num = s->fifo_next - s->fifo_stop; if (num < 0) { - num += CMD(max) - CMD(min); + num += s->fifo_max - s->fifo_min; } return num >> 2; } static inline uint32_t vmsvga_fifo_read_raw(struct vmsvga_state_s *s) { - uint32_t cmd = s->fifo[CMD(stop) >> 2]; + uint32_t cmd = s->fifo[s->fifo_stop >> 2]; - s->cmd->stop = cpu_to_le32(CMD(stop) + 4); - if (CMD(stop) >= CMD(max)) { - s->cmd->stop = s->cmd->min; + s->fifo_stop += 4; + if (s->fifo_stop >= s->fifo_max) { + s->fifo_stop = s->fifo_min; } + s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop); return cmd; } @@ -607,7 +605,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) len = vmsvga_fifo_length(s); while (len > 0) { /* May need to go back to the start of the command if incomplete */ - cmd_start = s->cmd->stop; + cmd_start = s->fifo_stop; switch (cmd = vmsvga_fifo_read(s)) { case SVGA_CMD_UPDATE: @@ -766,7 +764,8 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) break; rewind: - s->cmd->stop = cmd_start; + s->fifo_stop = cmd_start; + s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop); break; } }