| Message ID | 20221219063456.2017996-6-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Proposal to make x86 IOMMU driver support configurable | expand |
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.
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
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
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(+)