| Message ID | 20230116070431.905594-6-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make x86 IOMMU driver support configurable | expand |
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@gmail.com> > --- > > 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.
On 16.01.2023 08:21, Xenia Ragiadakou wrote: > 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. It would probably help if you mentioned here explicitly that, while affected, we don't handle that yet (the code setting the flag would either need to move out of VT-d specific space, or be cloned accordingly). >> 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@gmail.com> >>[...] > > 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. > With that added (and preferably the description clarified) Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 1/16/23 18:49, Jan Beulich wrote: > On 16.01.2023 08:21, Xenia Ragiadakou wrote: >> 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. > > It would probably help if you mentioned here explicitly that, while affected, > we don't handle that yet (the code setting the flag would either need to > move out of VT-d specific space, or be cloned accordingly). Sure. I will update the comment. > >>> 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@gmail.com> >>> [...] >> >> 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. >> > > With that added (and preferably the description clarified) > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Jan
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
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@gmail.com> --- 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(-)