Re: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Sun, 2011-05-22 at 19:15 +0100, Tim Deegan wrote:
> At 18:19 +0100 on 20 May (1305915548), Ian Jackson wrote:
> > Tim Deegan writes ("Re: [Xen-devel] Xen security advisory CVE-2011-1898 - 
> > VT-d (PCI passthrough) MSI"):
> > > At 21:48 +0100 on 19 May (1305841716), Cihula, Joseph wrote:
> > > > So how would the user (or installation SW) specify to use the best
> > > > (IOMMU) security available on the platform?
> > > 
> > > iommu=on.  That pretty much lines up with the current meaining. 
> > > 
> > > Only iommu=force requires a fully secure IOMMU, and you can
> > > overide that with iommu=force,nointremap.  
> > 
> > I think this is the best behaviour.  Do we have a patch that
> > implements it ?  If I'm not confused, the patch further upthread
> > crashes on lack of intremap even with iommu=on.
> 
> AIUI Ian Campbell's most recent patch does exactly this.  Ian?

You mean the first one in
<1305708848.20907.109.camel@xxxxxxxxxxxxxxxxxxxxxx>? I believe it has
this effect, yes, although the iommu setup code has several places which
fallback to intremap = 0 if some condition is not met e.g. "!
ecap_intr_remap(iommu->ecap)" or if "Queue Invalidation" is not
enabled/supported, I expect they need a similar check. Rather than
sprinkling panic()s everywhere lets just pull the check for force && !
intremap out into the top level generic function.

There also seemed to be a missing "iommu_intremap = 0" in the case where
enable_intremap failed in init_vtd_hw.

Ian.

8<----------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1306141240 -3600
# Node ID aafc09d804172e3169b64e0ae8f4fc56a1a70fbb
# Parent  3db330334e3512a5326bbed4881718a63eec171a
IOMMU: Fail if intremap is not available and iommu=required/force.

Rather than sprinkling panic()s throughout the setup code hoist the check up
into common code.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r 3db330334e35 -r aafc09d80417 xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c   Fri May 13 14:41:18 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c   Mon May 23 10:00:40 2011 +0100
@@ -311,6 +311,7 @@ int deassign_device(struct domain *d, u8
 int __init iommu_setup(void)
 {
     int rc = -ENODEV;
+    int need_intremap = iommu_intremap;
 
     if ( iommu_dom0_strict )
         iommu_passthrough = 0;
@@ -322,7 +323,9 @@ int __init iommu_setup(void)
     }
 
     if ( force_iommu && !iommu_enabled )
-        panic("IOMMU setup failed, crash Xen for security purpose!\n");
+        panic("Unable to enable IOMMU and iommu=required/force\n");
+    if ( force_iommu && !iommu_intremap && need_intremap )
+        panic("Unable to enable Interrupt Remapping and 
iommu=required/force\n");
 
     if ( !iommu_enabled )
     {
diff -r 3db330334e35 -r aafc09d80417 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c       Fri May 13 14:41:18 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c       Mon May 23 10:00:40 2011 +0100
@@ -1971,8 +1971,6 @@ static int init_vtd_hw(void)
                     "ioapic_to_iommu: ioapic 0x%x (id: 0x%x) is NULL! "
                     "Will not try to enable Interrupt Remapping.\n",
                     apic, IO_APIC_ID(apic));
-                if ( force_iommu )
-                    panic("intremap remapping failed to enable with 
iommu=required/force in grub\n");
                 break;
             }
         }
@@ -1984,11 +1982,10 @@ static int init_vtd_hw(void)
             iommu = drhd->iommu;
             if ( enable_intremap(iommu, 0) != 0 )
             {
+                iommu_intremap = 0;
                 dprintk(XENLOG_WARNING VTDPREFIX,
                         "Interrupt Remapping not enabled\n");
 
-                if ( force_iommu && platform_supports_intremap() )
-                    panic("intremap remapping failed to enable with 
iommu=required/force in grub\n");
                 break;
             }
         }





_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel