[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/4] x86/P2M: relax guarding of MMIO entries


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 30 Aug 2021 15:02:50 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=A0SFd/SWBGb/mVP/UiCsyrSrRP8U1K0mXERHcQ8qcR4=; b=oQVVkSQtUJkmhkZh0x8s7dH3LaYui/RRpvJ3jNPDCeE8Iw2BQ9VdVTtF0SunKbeKK9Ej3bcvMHB+e/ntdZWfdx8far0cX1PdZrqs3TexXgAZXI7ubQJEinXW7B0eqnAJKBAFWmaMvRnLOuPk7/SAC3r2B2suGr5V0uxwLGZEx29j32i2XNAJH3ZpKwfG70CPLPoLXBcrIdhRYF6wKbclsyJqylYNZpGxcdEfj2k4qOYlpFe0utXa7YOoG/7IJn4IAeeIMXdT1BjSUcpWLiFqNeWOKMeDZXB3JRBYPTCMQNnuQNlcq7Nd1Sw8pZCf8xe1R748VRw1NbsER3JmlsGIYw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SxzgXWeIbNEKjFTjHGWUnMJg/sdLTyVGWeCBGsPfxqCXLtIC0DbNACY5t4mI4uSt4ZNk8tmwv01ewYVX6dIONFPgU4TnUqrk3raTdBBftUq4ofr8DJ3NjXwZiyOeTTq2rk3GkRf4RTXL5WAMHUWgSrMr9NrKeQpolq9qaeavim5Inwc4NwFjdrVBSwkXFud5DhaJix9tS6qQfphJd4v3IImED8ERLCwGd7rE4CSeIyv0QoAJuFAeyswE/sNxeHt+Ne0ZZ7hfWWI78bCiGdl+XCAql3ljBMDHfLvvnXBt7QMBHiI8dMOSx1Oo/qorUFfBNXJEILdHKDpb76tpGN/ASA==
  • Authentication-results: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 30 Aug 2021 13:02:57 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

One of the changes comprising the fixes for XSA-378 disallows replacing
MMIO mappings by unintended (for this purpose) code paths. At least in
the case of PVH Dom0 hitting an RMRR covered by an E820 ACPI region,
this is too strict. Generally short-circuit requests establishing the
same kind of mapping that's already in place.

Further permit "access" to differ in the "executable" attribute. While
ideally only ROM regions would get mapped with X set, getting there is
quite a bit of work. Therefore, as a temporary measure, permit X to
vary. For Dom0 the more permissive of the types will be used, while for
DomU it'll be the more restrictive one.

While there, also add a log message to the other domain_crash()
invocation that did prevent PVH Dom0 from coming up after the XSA-378
changes.

Fixes: 753cb68e6530 ("x86/p2m: guard (in particular) identity mapping entries")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -958,9 +958,13 @@ guest_physmap_add_entry(struct domain *d
         if ( p2m_is_special(ot) )
         {
             /* Don't permit unmapping grant/foreign/direct-MMIO this way. */
-            domain_crash(d);
             p2m_unlock(p2m);
-            
+            printk(XENLOG_G_ERR
+                   "%pd: GFN %lx (%lx:%u:%u) -> (%lx:%u:%u) not permitted\n",
+                   d, gfn_x(gfn) + i,
+                   mfn_x(omfn), ot, a,
+                   mfn_x(mfn) + i, t, p2m->default_access);
+            domain_crash(d);
             return -EPERM;
         }
         else if ( p2m_is_ram(ot) && !p2m_is_paged(ot) )
@@ -1302,9 +1306,50 @@ static int set_typed_p2m_entry(struct do
     }
     if ( p2m_is_special(ot) )
     {
-        gfn_unlock(p2m, gfn, order);
-        domain_crash(d);
-        return -EPERM;
+        bool done = false, bad = true;
+
+        /* Special-case (almost) identical mappings. */
+        if ( mfn_eq(mfn, omfn) && gfn_p2mt == ot )
+        {
+            /*
+             * For MMIO allow X to differ in the requests (to cover for
+             * set_identity_p2m_entry() and set_mmio_p2m_entry() differing in
+             * the way they specify "access"). For the hardware domain put (or
+             * leave) in place the more permissive of the two possibilities,
+             * while for DomU-s go with the more restrictive variant.
+             */
+            if ( gfn_p2mt == p2m_mmio_direct &&
+                 access <= p2m_access_rwx &&
+                 (access ^ a) == p2m_access_x )
+            {
+                if ( is_hardware_domain(d) )
+                    access |= p2m_access_x;
+                else
+                    access &= ~p2m_access_x;
+                bad = access == p2m_access_n;
+            }
+
+            if ( access == a )
+                done = true;
+        }
+
+        if ( done )
+        {
+            gfn_unlock(p2m, gfn, order);
+            return 0;
+        }
+
+        if ( bad )
+        {
+            gfn_unlock(p2m, gfn, order);
+            printk(XENLOG_G_ERR
+                   "%pd: GFN %lx (%lx:%u:%u:%u) -> (%lx:%u:%u:%u) not 
permitted\n",
+                   d, gfn_l,
+                   mfn_x(omfn), cur_order, ot, a,
+                   mfn_x(mfn), order, gfn_p2mt, access);
+            domain_crash(d);
+            return -EPERM;
+        }
     }
     else if ( p2m_is_ram(ot) )
     {




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.