From patchwork Wed Jan 25 16:08:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 9537395 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 7B5FC6042C for ; Wed, 25 Jan 2017 16:11:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6C48427D5E for ; Wed, 25 Jan 2017 16:11:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6060D27D85; Wed, 25 Jan 2017 16:11:43 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9087227D8D for ; Wed, 25 Jan 2017 16:11:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cWQ8F-00081x-A5; Wed, 25 Jan 2017 16:09:11 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cWQ8E-00081r-41 for xen-devel@lists.xen.org; Wed, 25 Jan 2017 16:09:10 +0000 Received: from [85.158.137.68] by server-9.bemta-3.messagelabs.com id 1A/77-12625-5ADC8885; Wed, 25 Jan 2017 16:09:09 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrAIsWRWlGSWpSXmKPExsUSNTvmou6Ssx0 RBgd/clks+biYxYHR4+ju30wBjFGsmXlJ+RUJrBkzevvZC7aEVdzceZy5gXFBUBcjF4eQwAZG iWnvOli6GDk5eAVMJXaf7GQCsYUFqiW2XPnACGKzCWhLHNhxEqxGREBZovfXbzCbWaBY4lLja bB6TgF7iU0nJjJDDJ3AKLHxWhsrRFGtxJKji4AaODhYBFQlHj9wBTF5BQQl/u4QBqmQENCQeN iyjhHCbmOUuLfabgIj7ywkzbMQOiDCmhKt23+zQ9jaEssWvmaGsG0l9l9dCWWbSrw++pERwla UmNL9kH0BI/sqRo3i1KKy1CJdI1O9pKLM9IyS3MTMHF1DA2O93NTi4sT01JzEpGK95PzcTYzA kK1nYGDcwdh6wu8QoyQHk5Io76lTHRFCfEn5KZUZicUZ8UWlOanFhxhlODiUJHjVzgDlBItS0 1Mr0jJzgNEDk5bg4FES4U0ESfMWFyTmFmemQ6ROMSpKifOmgiQEQBIZpXlwbbCIvcQoKyXMy8 jAwCDEU5BalJtZgir/ilGcg1FJmNcRZApPZl4J3PRXQIuZgBZfYG4HWVySiJCSamAsuHJ1npH MmTjxlMq4R0mJKyoiny+8n3uw+dCWtj0rdr0sX5RWeeCX4Nbds1Zrz9gclbhUX423NPL4qWg1 kYC2GMbVphWLeJR7FqXGs3BlPoz051qsonKDTU7lk8Ym9qB2tqc7tfNu/blw8W0At+HrJyEGm owbdh79O23X2a9Lph4WX3H/ZPAvJZbijERDLeai4kQAsY/1qdMCAAA= X-Env-Sender: BATV+800ed4118cbc253f6339+4903+infradead.org+dwmw2@twosheds .srs.infradead.org X-Msg-Ref: server-4.tower-31.messagelabs.com!1485360547!24278753!1 X-Originating-IP: [90.155.92.209] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.1.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 57299 invoked from network); 25 Jan 2017 16:09:08 -0000 Received: from twosheds.infradead.org (HELO twosheds.infradead.org) (90.155.92.209) by server-4.tower-31.messagelabs.com with AES256-GCM-SHA384 encrypted SMTP; 25 Jan 2017 16:09:08 -0000 Received: from [2001:8b0:10b:1:3cf1:aac7:7113:5830] by twosheds.infradead.org with esmtpsa (Exim 4.87 #1 (Red Hat Linux)) id 1cWQ7y-00075v-HU; Wed, 25 Jan 2017 16:08:54 +0000 Message-ID: <1485360534.4727.127.camel@infradead.org> From: David Woodhouse To: Jan Beulich In-Reply-To: <5888C2810200007800133CDC@prv-mh.provo.novell.com> References: <1485353329.4727.111.camel@infradead.org> <5888C2810200007800133CDC@prv-mh.provo.novell.com> Date: Wed, 25 Jan 2017 16:08:54 +0000 Mime-Version: 1.0 X-Mailer: Evolution 3.18.5.2-0ubuntu3.1 X-SRS-Rewrite: SMTP reverse-path rewritten from by twosheds.infradead.org. See http://www.infradead.org/rpr.html Cc: Andrew Cooper , "H. Peter Anvin" , xen-devel@lists.xen.org Subject: Re: [Xen-devel] Xen Security Advisory 154 (CVE-2016-2270) - x86: inconsistent cachability flags on guest mappings X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2017-01-25 at 07:21 -0700, Jan Beulich wrote: > > If there wasn't INVALID_MFN to be taken care of, the !mfn_valid() > check could simply move down, and be combined with the > direct_mmio one. As discussed on IRC, once we fix the overflow with order > 0, I think INVALID_MFN is OK. Not that it should ever really happen, AFAICT.  This seems to do the right thing for my MMIO WC test. I haven't actually combined the !mfn_valid() check with the direct_mmio one. Under what circumstances does that make sense anyway? For now in the patch below I've left it *forcing* UC, unlike the direct_mmio path which lets the guest use WC. But really, shouldn't the '!direct_mmio && !mfn_valid()' case just return an error? Note that as well as using a mask for 'order' I've attempted to consolidate the unlikely rangeset_overlaps_range() and rangeset_contains_range() calls, on the assumption that the former will *always* be true if the latter is, so we only need one of them in the fast path through the function. diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 709759c..09c2f5c 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -773,20 +773,24 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,      if ( v->domain != d )          v = d->vcpu ? d->vcpu[0] : NULL;   -    if ( !mfn_valid(mfn_x(mfn)) || -         rangeset_contains_range(mmio_ro_ranges, mfn_x(mfn), -                                 mfn_x(mfn) + (1UL << order) - 1) ) +    /* INVALID_MFN should never happen here, but if it does then this +     * should do the right thing instead of exploding */ +    if ( unlikely(rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn), +   mfn_x(mfn) | ((1UL << order) - 1))) )      { -        *ipat = 1; -        return MTRR_TYPE_UNCACHABLE; + if ( rangeset_contains_range(mmio_ro_ranges, mfn_x(mfn), +      mfn_x(mfn) | ((1UL << order) - 1)) ) + { +     *ipat = 1; +     return MTRR_TYPE_UNCACHABLE; + } + /* Overlaps mmio_ro_ranges but is not entirely contained. No. */ + return -1;      }   -    if ( rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn), -                                 mfn_x(mfn) + (1UL << order) - 1) ) -        return -1; -      if ( direct_mmio )      { + /* Again, INVALID_MFN should do the right thing here. */          if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order )              return MTRR_TYPE_UNCACHABLE;          if ( order ) @@ -795,6 +799,12 @@ int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,          return MTRR_TYPE_WRBACK;      }   +    if ( unlikely(!mfn_valid(mfn_x(mfn))) ) +    { + *ipat = 1; + return MTRR_TYPE_UNCACHABLE; +    } +      if ( !need_iommu(d) && !cache_flush_permitted(d) )      {          *ipat = 1;