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 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@xxxxxxxxx>
> > > [...]
> > 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@xxxxxxxx>
>
> Jan
--
Xenia