Xen Security Advisory 450 v2 (CVE-2023-46840) - VT-d: Failure to quarantine devices in !HVM builds

Xen.org security team posted 1 patch 2 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/E1rUnvX-000489-JF@xenbits.xenproject.org
Xen Security Advisory 450 v2 (CVE-2023-46840) - VT-d: Failure to quarantine devices in !HVM builds
Posted by Xen.org security team 2 months, 3 weeks ago
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2023-46840 / XSA-450
                               version 2

           VT-d: Failure to quarantine devices in !HVM builds

UPDATES IN VERSION 2
====================

Public release.

ISSUE DESCRIPTION
=================

Incorrect placement of a preprocessor directive in source code results
in logic that doesn't operate as intended when support for HVM guests is
compiled out of Xen.

IMPACT
======

When a device is removed from a domain, it is not properly quarantined
and retains its access to the domain to which it was previously
assigned.

VULNERABLE SYSTEMS
==================

Xen 4.17 and onwards are vulnerable.  Xen 4.16 and older are not
vulnerable.

Only Xen running on x86 platforms with an Intel-compatible VT-d IOMMU is
vulnerable.  Platforms from other manufacturers, or platforms without a
VT-d IOMMU are not vulnerable.

Only systems where PCI devices are passed through to untrusted or
semi-trusted guests are vulnerable.  Systems which do not assign PCI
devices to untrusted guests are not vulnerable.

Xen is only vulnerable when CONFIG_HVM is disabled at build time.  Most
deployments of Xen are expected to have CONFIG_HVM enabled at build
time, and would therefore not be vulnerable.

MITIGATION
==========

There is no mitigation.

CREDITS
=======

This issue was discovered by Teddy Astie of Vates

RESOLUTION
==========

Applying the attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa450.patch           xen-unstable - Xen 4.17.x

$ sha256sum xsa450*
738c79b92ab5ea57f446df3daff6564727fea5feebf8fadeb32acd0cf06ff9fb  xsa450.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmW49MwMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZnwcIALs07CQFYSuQmdgWRYeepkjehMSVhPhvJcYBCFWU
p+80oreGP2pC1LN+T9ndN0kDeUHAO8PeT+XqxHSNfT16Q5EOSeLpUQ8m+UfHUFLU
vtPMjR4sMpnvuZfx0OCMJctDDTM+/muw4AH0BO2zxFfDzGkM96zZ6vAokeer+5HQ
/P9usMm/6jphixVq919RBJ78fFZxKpKhil9tEwNuD6HJW3VNMWp1ypGNyFI3iRhw
XpYzWMB0eW6B6rSInohHJiTS7P6KE5zeXeBPZ5yVHy2J3e3c7nXyrQaaONSRCBdm
/Px2xcg1SpH+3UwoT56Z7tj1DhlgjcY4peb5B58oDK68hMU=
=Dp+G
-----END PGP SIGNATURE-----
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: VT-d: Fix "else" vs "#endif" misplacement

In domain_pgd_maddr() the "#endif" is misplaced with respect to "else".  This
generates incorrect logic when CONFIG_HVM is compiled out, as the "else" body
is executed unconditionally.

Rework the logic to use IS_ENABLED() instead of explicit #ifdef-ary, as it's
clearer to follow.  This in turn involves adjusting p2m_get_pagetable() to
compile when CONFIG_HVM is disabled.

This is XSA-450 / CVE-2023-46840.

Reported-by: Reported-by: Teddy Astie <teddy.astie@vates.tech>
Fixes: 033ff90aa9c1 ("x86/P2M: p2m_{alloc,free}_ptp() and p2m_alloc_table() are HVM-only")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 32f3f394b05a..6ada585eaac2 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -435,7 +435,14 @@ static inline bool p2m_is_altp2m(const struct p2m_domain *p2m)
     return p2m->p2m_class == p2m_alternate;
 }
 
-#define p2m_get_pagetable(p2m)  ((p2m)->phys_table)
+#ifdef CONFIG_HVM
+static inline pagetable_t p2m_get_pagetable(const struct p2m_domain *p2m)
+{
+    return p2m->phys_table;
+}
+#else
+pagetable_t p2m_get_pagetable(const struct p2m_domain *p2m);
+#endif
 
 /*
  * Ensure any deferred p2m TLB flush has been completed on all VCPUs.
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 99b642f12ef9..4244855032ee 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -438,15 +438,13 @@ static paddr_t domain_pgd_maddr(struct domain *d, paddr_t pgd_maddr,
 
     if ( pgd_maddr )
         /* nothing */;
-#ifdef CONFIG_HVM
-    else if ( iommu_use_hap_pt(d) )
+    else if ( IS_ENABLED(CONFIG_HVM) && iommu_use_hap_pt(d) )
     {
         pagetable_t pgt = p2m_get_pagetable(p2m_get_hostp2m(d));
 
         pgd_maddr = pagetable_get_paddr(pgt);
     }
     else
-#endif
     {
         if ( !hd->arch.vtd.pgd_maddr )
         {