The variable untrusted_msi indicates whether the system is vulnerable to
CVE-2011-1898. This vulnerablity is VT-d specific.
Place the code that addresses the issue under CONFIG_INTEL_VTD.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/arch/x86/include/asm/iommu.h | 2 ++
 xen/arch/x86/pv/hypercall.c      | 2 ++
 xen/arch/x86/x86_64/entry.S      | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h
index fc0afe35bf..41bd1b9e05 100644
--- a/xen/arch/x86/include/asm/iommu.h
+++ b/xen/arch/x86/include/asm/iommu.h
@@ -127,7 +127,9 @@ int iommu_identity_mapping(struct domain *d, p2m_access_t p2ma,
                            unsigned int flag);
 void iommu_identity_map_teardown(struct domain *d);
 
+#ifdef CONFIG_INTEL_VTD
 extern bool untrusted_msi;
+#endif
 
 int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
                    const uint8_t gvec);
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c
index 2eedfbfae8..0e1b03904c 100644
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -193,8 +193,10 @@ void pv_ring1_init_hypercall_page(void *p)
 
 void do_entry_int82(struct cpu_user_regs *regs)
 {
+#ifdef CONFIG_INTEL_VTD
     if ( unlikely(untrusted_msi) )
         check_for_unexpected_msi((uint8_t)regs->entry_vector);
+#endif
 
     _pv_hypercall(regs, true /* compat */);
 }
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index ae01285181..2e06c0a6c1 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -406,11 +406,13 @@ ENTRY(int80_direct_trap)
 .Lint80_cr3_okay:
         sti
 
+#ifdef CONFIG_INTEL_VTD
         cmpb  $0,untrusted_msi(%rip)
 UNLIKELY_START(ne, msi_check)
         movl  $0x80,%edi
         call  check_for_unexpected_msi
 UNLIKELY_END(msi_check)
+#endif
 
         movq  STACK_CPUINFO_FIELD(current_vcpu)(%rbx), %rbx
 
-- 
2.37.2

On 19/12/2022 6:34 am, Xenia Ragiadakou wrote:
> The variable untrusted_msi indicates whether the system is vulnerable to
> CVE-2011-1898. This vulnerablity is VT-d specific.
> Place the code that addresses the issue under CONFIG_INTEL_VTD.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Actually, this variable is pretty bogus.  I think I'd like to delete it
entirely.

There are systems with no IOMMU at all, and we certainly used to let PV
Passthrough go ahead.  (Not sure we do any more.)

There are systems with DMA remapping only, but no interrupt remapping. 
These are known insecure.  I'm honestly not convinced that an ISR read
and crash is useful when the user has already constructed an
known-unsafe configuration, because a malicious guest in that case can
still fully mess with dom0 by sending vectors other than 0x80 and 0x82.

In particular, this option does not get activated on AMD when the user
elects to disable interrupt remapping, and I'm disinclined to wire it up
in that case too.

~Andrew

P.S. It occurs to me that FRED obsoletes the need for this anyway,
seeing as it does properly distinguish the source of an event.

On 12/20/22 13:09, Andrew Cooper wrote:
> On 19/12/2022 6:34 am, Xenia Ragiadakou wrote:
>> The variable untrusted_msi indicates whether the system is vulnerable to
>> CVE-2011-1898. This vulnerablity is VT-d specific.
>> Place the code that addresses the issue under CONFIG_INTEL_VTD.
>>
>> No functional change intended.
>>
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> 
> Actually, this variable is pretty bogus.  I think I'd like to delete it
> entirely.

Nevertheless, I don't think that it would be appropriate to be done as 
part of this series.

> 
> There are systems with no IOMMU at all, and we certainly used to let PV
> Passthrough go ahead.  (Not sure we do any more.)
> 
> There are systems with DMA remapping only, but no interrupt remapping.
> These are known insecure.  I'm honestly not convinced that an ISR read
> and crash is useful when the user has already constructed an
> known-unsafe configuration, because a malicious guest in that case can
> still fully mess with dom0 by sending vectors other than 0x80 and 0x82.
> 
> In particular, this option does not get activated on AMD when the user
> elects to disable interrupt remapping, and I'm disinclined to wire it up
> in that case too.
> 
> ~Andrew
> 
> P.S. It occurs to me that FRED obsoletes the need for this anyway,
> seeing as it does properly distinguish the source of an event.

-- 
Xenia

On 21.12.2022 16:22, Xenia Ragiadakou wrote:
> 
> On 12/20/22 13:09, Andrew Cooper wrote:
>> On 19/12/2022 6:34 am, Xenia Ragiadakou wrote:
>>> The variable untrusted_msi indicates whether the system is vulnerable to
>>> CVE-2011-1898. This vulnerablity is VT-d specific.
>>> Place the code that addresses the issue under CONFIG_INTEL_VTD.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>
>> Actually, this variable is pretty bogus.  I think I'd like to delete it
>> entirely.

The important difference between Intel and AMD was that Intel initially
supplied DMA-remap-only IOMMUs, while AMD had intremap from the beginning.
Hence Intel hardware could be unsafe by default, whereas on AMD an admin
would need to come and turn off intremap. Deleting the variable would be
okay only if we declared Xen security-unsupported on inremap-less Intel
hardware. Extending coverage to AMD wouldn't seem unreasonable to me, if
we knew that there were people turning off intremap _and_ caring about
this particular class of attack. With no-one having complained in over
10 years, perhaps there's no-one of this kind ...

> Nevertheless, I don't think that it would be appropriate to be done as 
> part of this series.

I agree, but I'll want to comment on v2 nevertheless, rather than simply
ack-ing it.

Jan