Re: [PATCH v3 5/8] x86/iommu: make code addressing CVE-2011-1898 no VT-d specific

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

On 1/16/23 09:04, Xenia Ragiadakou wrote:
> The variable untrusted_msi indicates whether the system is vulnerable to
> CVE-2011-1898 due to the absence of interrupt remapping  support.
> AMD iommus with interrupt remapping disabled are also exposed.
> Therefore move the definition of the variable to the common x86 iommu code.
>
> Also, since the current implementation assumes that only PV guests are prone
> to this attack, take the opportunity to define untrusted_msi only when PV is
> enabled.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> ---
>
> Changes in v3:
>    - change untrusted_msi from being VT-d specific to PV specific and
>      update commit log accordingly
>    - remove unnecessary #ifdef guard from its declaration
>
>   xen/drivers/passthrough/vtd/iommu.c | 3 ---
>   xen/drivers/passthrough/x86/iommu.c | 5 +++++
>   2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
> b/xen/drivers/passthrough/vtd/iommu.c
> index 62e143125d..dae2426cb9 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -54,9 +54,6 @@
>                                    ? dom_iommu(d)->arch.vtd.pgd_maddr \
>                                    : (pdev)->arch.vtd.pgd_maddr)
>   
> -/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
> -bool __read_mostly untrusted_msi;
> -
>   bool __read_mostly iommu_igfx = true;
>   bool __read_mostly iommu_qinval = true;
>   #ifndef iommu_snoop
> diff --git a/xen/drivers/passthrough/x86/iommu.c 
> b/xen/drivers/passthrough/x86/iommu.c
> index f671b0f2bb..c5021ea023 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -36,6 +36,11 @@ bool __initdata iommu_superpages = true;
>   
>   enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
>   
> +#ifdef CONFIG_PV
> +/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
> +bool __read_mostly untrusted_msi;
> +#endif
> +
>   #ifndef iommu_intpost
>   /*
>    * In the current implementation of VT-d posted interrupts, in some extreme
Here, somehow I missed the part:
diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index dae2426cb9..e97b1fe8cd 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2767,6 +2767,7 @@ static int cf_check reassign_device_ownership(
         if ( !has_arch_pdevs(target) )
             vmx_pi_hooks_assign(target);

+#ifdef CONFIG_PV
         /*
          * Devices assigned to untrusted domains (here assumed to be 
any domU)
          * can attempt to send arbitrary LAPIC/MSI messages. We are 
unprotected
@@ -2775,6 +2776,7 @@ static int cf_check reassign_device_ownership(
         if ( !iommu_intremap && !is_hardware_domain(target) &&
              !is_system_domain(target) )
             untrusted_msi = true;
+#endif

         ret = domain_context_mapping(target, devfn, pdev);

I will fix it in the next version.

--
Xenia