From patchwork Thu Feb 9 13:02:20 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 9564535 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 475D5601C3 for ; Thu, 9 Feb 2017 13:20:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2DA4F284F9 for ; Thu, 9 Feb 2017 13:20:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1F955284FB; Thu, 9 Feb 2017 13:20:04 +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 E92C9284F9 for ; Thu, 9 Feb 2017 13:20:02 +0000 (UTC) Received: from localhost ([::1]:37869 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cbodl-0007PG-Gy for patchwork-qemu-devel@patchwork.kernel.org; Thu, 09 Feb 2017 08:20:01 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cboN4-0001Pb-KV for qemu-devel@nongnu.org; Thu, 09 Feb 2017 08:02:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cboN1-0002Nk-Pv for qemu-devel@nongnu.org; Thu, 09 Feb 2017 08:02:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37502) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cboN1-0002NR-Id for qemu-devel@nongnu.org; Thu, 09 Feb 2017 08:02:43 -0500 Received: from smtp.corp.redhat.com (int-mx16.intmail.prod.int.phx2.redhat.com [10.5.11.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AA36C1B174B; Thu, 9 Feb 2017 13:02:43 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-117-196.ams2.redhat.com [10.36.117.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 33BA16377B; Thu, 9 Feb 2017 13:02:42 +0000 (UTC) Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id CA7DE80C25; Thu, 9 Feb 2017 14:02:35 +0100 (CET) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Thu, 9 Feb 2017 14:02:20 +0100 Message-Id: <1486645341-5010-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.74 on 10.5.11.28 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 09 Feb 2017 13:02:43 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 1/2] cirrus: fix patterncopy checks 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: Wolfgang Bumiller , Gerd Hoffmann , "Dr. David Alan Gilbert" Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP The blit_region_is_unsafe checks don't work correctly for the patterncopy source. It's a fixed-sized region, which doesn't depend on cirrus_blt_{width,height}. So go do the check in cirrus_bitblt_common_patterncopy instead, then tell blit_is_unsafe that it doesn't need to verify the source. Also handle the case where we blit from cirrus_bitbuf correctly. This patch replaces 5858dd1801883309bdd208d72ddb81c4e9fee30c. Security impact: I think for the most part error on the safe side this time, refusing blits which should have been allowed. Only exception is placing the blit source at the end of the video ram, so cirrus_blt_srcaddr + 256 goes beyond the end of video memory. But even in that case I'm not fully sure this actually allows read access to host memory. To trick the commit 5858dd18 security checks one has to pick very small cirrus_blt_{width,height} values, which in turn implies only a fraction of the blit source will actually be used. Cc: Wolfgang Bumiller Cc: Dr. David Alan Gilbert Signed-off-by: Gerd Hoffmann Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Wolfgang Bumiller Reviewed-by: Laurent Vivier --- hw/display/cirrus_vga.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 16f27e8..6bd13fc 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -683,14 +683,39 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin, } } -static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s, - const uint8_t * src) +static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc) { + uint32_t patternsize; uint8_t *dst; + uint8_t *src; dst = s->vga.vram_ptr + s->cirrus_blt_dstaddr; - if (blit_is_unsafe(s, false, true)) { + if (videosrc) { + switch (s->vga.get_bpp(&s->vga)) { + case 8: + patternsize = 64; + break; + case 15: + case 16: + patternsize = 128; + break; + case 24: + case 32: + default: + patternsize = 256; + break; + } + s->cirrus_blt_srcaddr &= ~(patternsize - 1); + if (s->cirrus_blt_srcaddr + patternsize > s->vga.vram_size) { + return 0; + } + src = s->vga.vram_ptr + s->cirrus_blt_srcaddr; + } else { + src = s->cirrus_bltbuf; + } + + if (blit_is_unsafe(s, true, true)) { return 0; } @@ -731,8 +756,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) static int cirrus_bitblt_videotovideo_patterncopy(CirrusVGAState * s) { - return cirrus_bitblt_common_patterncopy(s, s->vga.vram_ptr + - (s->cirrus_blt_srcaddr & ~7)); + return cirrus_bitblt_common_patterncopy(s, true); } static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h) @@ -831,7 +855,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * s) if (s->cirrus_srccounter > 0) { if (s->cirrus_blt_mode & CIRRUS_BLTMODE_PATTERNCOPY) { - cirrus_bitblt_common_patterncopy(s, s->cirrus_bltbuf); + cirrus_bitblt_common_patterncopy(s, false); the_end: s->cirrus_srccounter = 0; cirrus_bitblt_reset(s);