RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Jan Beulich]

[-- Attachment #1.1: Type: text/plain, Size: 1866 bytes --]

>>> "Cihula, Joseph"  05/16/11 11:34 PM >>>
>IOMMU adds security capabilities.  IR adds additional security capabilities.  IOMMU allows for fully isolating the hypervisor
>from domains even if the domains control DMA devices.  It helps to protect against buggy drivers or device FW by limiting
>the areas such bugs can affect to just the DMA data buffers.  IOMMU, in conjunction with Intel(R) Trusted Execution
>Technology (TXT), provides DMA protection through the entire launch process and into runtime.  This is all true regardless
>of the presence of IR.  IR adds the ability to prevent DoS attacks by untrusted domains with assigned DMA devices,
>malicious device FW, etc.  This is incremental--not all or nothing.

I think this is the problem - you're saying things like "fully isolating" and "regardless of the presence of IR", while the paper they made accessible meanwhile makes clear that neither is true. Thus the mere presence of DMA protection creates false expectation of customers - without IR (and with MSI supported by the system, not necessarily the device passed through) there's no way for isolation to become complete (actually, with non-MSI-capable devices or by disallowing MSI altogether on capable ones, depending of whether MSI writes bypass the IOMMU or simply get 1:1 translated, it could be possible to make this secure).

>The 00-block-msis-on-trap-vectors patch (esp. in conjunction with TXT) prevents all known security exploits of MSI misuse.

All? Not really, just a very small subset, and only partially. The SIPI one is perhaps the worst case (not prevented by this patch), but being able to send SMI or NMI perhaps isn't much better (as long as we're considering DoS attacks to also be a problem, which at least I do, and in which case said patch only converts from one [worse] to another ["better"] evil).

Jan


[-- Attachment #1.2: HTML --]
[-- Type: text/html, Size: 2186 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

[-- Attachment #1.1: Type: text/plain, Size: 3338 bytes --]

From: Jan Beulich [mailto:jbeulich@novell.com]
Sent: Tuesday, May 17, 2011 12:43 AM
To: Ian.Campbell@citrix.com; Cihula, Joseph
Cc: xen-devel@lists.xensource.com
Subject: RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

>>> "Cihula, Joseph" <joseph.cihula@intel.com<mailto:joseph.cihula@intel.com>> 05/16/11 11:34 PM >>>
>IOMMU adds security capabilities. IR adds additional security capabilities. IOMMU allows for fully isolating the hypervisor
>from domains even if the domains control DMA devices. It helps to protect against buggy drivers or device FW by limiting
>the areas such bugs can affect to just the DMA data buffers. IOMMU, in conjunction with Intel(R) Trusted Execution
>Technology (TXT), provides DMA protection through the entire launch process and into runtime. This is all true regardless
>of the presence of IR. IR adds the ability to prevent DoS attacks by untrusted domains with assigned DMA devices,
>malicious device FW, etc. This is incremental--not all or nothing.

I think this is the problem - you're saying things like "fully isolating" and "regardless of the presence of IR", while the paper they made accessible
[JC]  I will admit to being a little overreaching in use of that adverb; let's remove "fully" and my statement still stands.

meanwhile makes clear that neither is true. Thus the mere presence of DMA protection creates false expectation of customers - without IR (and with MSI supported by the system, not necessarily the device passed through) there's no way for isolation to become complete (actually, with non-MSI-capable devices or by disallowing MSI altogether on capable ones, depending of whether MSI writes bypass the IOMMU or simply get 1:1 translated, it could be possible to make this secure).
[JC]  The expectations of users will depend on the features advertised and how they are implemented in the SW solutions that use IOMMU.  Such SW should not be advertising DoS protection if devices are assigned to untrusted domains on systems without IR.  Just like it should not advertise that on systems without IOMMU.  (Note that the DMA protection of IOMMU should prevent most/all accidental/buggy behaviors; MSI DoS would almost certainly have to be due to malicious code.)

>The 00-block-msis-on-trap-vectors patch (esp. in conjunction with TXT) prevents all known security exploits of MSI misuse.

All? Not really, just a very small subset, and only partially. The SIPI one is perhaps the worst case (not prevented by this patch), but being able to send SMI or NMI perhaps isn't much better (as long as we're considering DoS attacks to also be a problem, which at least I do, and in which case said patch only converts from one [worse] to another ["better"] evil).
[JC]  I didn't say "all exploits"-I said all *known* exploits, and that is correct.  The SIPI attack, which isn't known to be exploitable, is prevented by TXT (hence the "esp. in conjunction with TXT" in my statement).  Naturally, DoS attacks are problematic from a reliability perspective but they were not what I was referring to with the term "security exploit".

I still fail to see an argument that justifies panic'ing Xen when IOMMU is enabled on non-IR systems.  And I don't think the way to manage customer expectations is through SW freezing.


Jan

[-- Attachment #1.2: Type: text/html, Size: 6816 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Campbell]

On Tue, 2011-05-17 at 23:52 +0100, Cihula, Joseph wrote:
> I still fail to see an argument that justifies panic’ing Xen when
> IOMMU is enabled on non-IR systems.  And I don’t think the way to
> manage customer expectations is through SW freezing.

Leaving the iommu=on behaviour the same as today but making iommu=force
require IR (with an explicit opt-out a la iommu=force,no-intremap) is
possibly sufficient (and considerably less harsh than the previous
proposal).

Perhaps something like the following? 

# HG changeset patch
# Parent 3db330334e3512a5326bbed4881718a63eec171a
diff -r 3db330334e35 -r 975cd3817d7a 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 16 11:37:41 2011 +0100
@@ -1987,7 +1987,7 @@ static int init_vtd_hw(void)
                 dprintk(XENLOG_WARNING VTDPREFIX,
                         "Interrupt Remapping not enabled\n");
 
-                if ( force_iommu && platform_supports_intremap() )
+                if ( force_iommu )
                     panic("intremap remapping failed to enable with iommu=required/force in grub\n");
                 break;
             }

I also considered 
+               if ( force_iommy && ( !tboot_in_measured_env() || platform_supports_intremap() )

but I trusting the result of tboot_in_measured_env() in the non-TXT case
seems a bit silly.

Ian.

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

> From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> Sent: Wednesday, May 18, 2011 1:54 AM
> 
> On Tue, 2011-05-17 at 23:52 +0100, Cihula, Joseph wrote:
> > I still fail to see an argument that justifies panic’ing Xen when
> > IOMMU is enabled on non-IR systems.  And I don’t think the way to
> > manage customer expectations is through SW freezing.
> 
> Leaving the iommu=on behaviour the same as today but making iommu=force require IR (with an
> explicit opt-out a la iommu=force,no-intremap) is possibly sufficient (and considerably less harsh
> than the previous proposal).
> 
> Perhaps something like the following?
> 
> # HG changeset patch
> # Parent 3db330334e3512a5326bbed4881718a63eec171a
> diff -r 3db330334e35 -r 975cd3817d7a 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 16 11:37:41 2011 +0100
> @@ -1987,7 +1987,7 @@ static int init_vtd_hw(void)
>                  dprintk(XENLOG_WARNING VTDPREFIX,
>                          "Interrupt Remapping not enabled\n");
> 
> -                if ( force_iommu && platform_supports_intremap() )
> +                if ( force_iommu )
>                      panic("intremap remapping failed to enable with iommu=required/force in
> grub\n");
>                  break;
>              }

So how would the user (or installation SW) specify to use the best (IOMMU) security available on the platform?  It looks like this change would again either mean giving up on forcing IOMMU or having to alter the command line based on knowing what the platform's IR capability is.

I think that it is desirable to have an option that says "use the best IOMMU security supported by the platform", and "force" does that right now.

Since what you're really trying to do is to always force IR to be on, why not just create a new option "force-intr"?  Or if your goal is to force the use of the best security capabilities available on any platform, and fail on the rest, then define a new option for that.

> 
> I also considered
> +               if ( force_iommy && ( !tboot_in_measured_env() ||
> + platform_supports_intremap() )
> 
> but I trusting the result of tboot_in_measured_env() in the non-TXT case seems a bit silly.

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Tim Deegan]

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.  

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

> From: Tim Deegan [mailto:Tim.Deegan@citrix.com]
> Sent: Friday, May 20, 2011 3:17 AM
> 
> 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.

'iommu=on' is really "*try* to use the best but it's OK if you can't", as it will allow execution to continue even if enabling the supported features fails.  'force' is really this with the caveat that execution won't continue if it fails to enable them.

> Only iommu=force requires a fully secure IOMMU, and you can overide that with
> iommu=force,nointremap.

But as I said, if you're writing a Xen installer and you want to *ensure* that Xen uses the HW features of the system, are you going to make some table that tells whether any given system supports IR so that you know which ones to add ',nointremap' to?  Because if you try to detect it at install time, malware could convince you to disable IR even though the HW supports it and TXT would report correctly to Xen.

What is the reason for not creating a new option that supports your desired behavior?

Joe
> 
> Tim.
> 
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9
> 0BG)

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

[Ian Jackson]

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.

Ian.

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

[Tim Deegan]

At 17:02 +0100 on 20 May (1305910924), Cihula, Joseph wrote:
> > 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.
> 
> 'iommu=on' is really "*try* to use the best but it's OK if you can't",
> as it will allow execution to continue even if enabling the supported
> features fails.  'force' is really this with the caveat that execution
> won't continue if it fails to enable them.

Yes, exactly.  By extension, "on" turns on interrupt remapping if it's
there but "it's OK if you can't"; "force" requires it.  I don't
understand why you would want iommu=force to crash if the IOMMU is
missing but not if it's insecure.

> But as I said, if you're writing a Xen installer and you want to
> *ensure* that Xen uses the HW features of the system, are you going to
> make some table that tells whether any given system supports IR so
> that you know which ones to add ',nointremap' to? 

This is exactly the behaviour we already have if you don't have an iommu
at all.  The installer already needs to figure out whether there's an
IOMMU, or make it optional.

If you really want to rely on TXT and Xen to mutuallly secure each
other, then as far as I can see you _need_ an interrupt remapper in all
your supported hardware.  That being the case, iommu=force should
enforce that requirement.  If you're willing to settle for best-effort,
iommu=on already does that.

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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

[Tim Deegan]

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?

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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

[Ian Campbell]

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@zakaz.uk.xensource.com>? 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@citrix.com>
# 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@citrix.com>

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;
             }
         }

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

> From: Tim Deegan [mailto:Tim.Deegan@citrix.com]
> Sent: Sunday, May 22, 2011 11:14 AM
> 
> At 17:02 +0100 on 20 May (1305910924), Cihula, Joseph wrote:
> > > 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.
> >
> > 'iommu=on' is really "*try* to use the best but it's OK if you can't",
> > as it will allow execution to continue even if enabling the supported
> > features fails.  'force' is really this with the caveat that execution
> > won't continue if it fails to enable them.
> 
> Yes, exactly.  By extension, "on" turns on interrupt remapping if it's there but "it's OK if you
> can't"; "force" requires it.  I don't understand why you would want iommu=force to crash if the
> IOMMU is missing but not if it's insecure.
> 
> > But as I said, if you're writing a Xen installer and you want to
> > *ensure* that Xen uses the HW features of the system, are you going to
> > make some table that tells whether any given system supports IR so
> > that you know which ones to add ',nointremap' to?
> 
> This is exactly the behaviour we already have if you don't have an iommu at all.  The installer
> already needs to figure out whether there's an IOMMU, or make it optional.
> 
> If you really want to rely on TXT and Xen to mutuallly secure each other, then as far as I can see
> you _need_ an interrupt remapper in all your supported hardware.  That being the case, iommu=force

Let me take one more shot at this, since no one has yet refuted my original points.

Why do you *need* IR to have a secure Xen w/ TXT?  Certainly a DoS is very undesirable, but that is not really a security issue.  Tell me what security exploits are still possible with the current patches.

If this all revolves around "gut feel" and not actual technical issues, then I suppose there isn't much more I can say.  But the "argument" that without IR "it seems like we could do more bad things" is purely speculative.  Likewise, the "argument" that "just because only the mitigated attack was presented doesn't mean that there aren't others possible" is also meaningless--this same reasoning applies to any attack, and so just because there is one buffer overflow found (e.g. CVE-2008-3687) do you panic Xen because there could be others that just haven't been found yet?

If someone can present a security issue that TXT and SW patches cannot mitigate, then I would agree that it is not possible to secure a system without IR and it should be forced on.  If you can't make this claim then I don't see how you have a technical basis for your position.

Joe

> should enforce that requirement.  If you're willing to settle for best-effort, iommu=on already
> does that.
> 
> Tim.
> 
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9
> 0BG)

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

[Tim Deegan]

At 22:35 +0100 on 23 May (1306190138), Cihula, Joseph wrote:
> > This is exactly the behaviour we already have if you don't have an iommu at all.  The installer
> > already needs to figure out whether there's an IOMMU, or make it optional.
> > 
> > If you really want to rely on TXT and Xen to mutuallly secure each other, then as far as I can see
> > you _need_ an interrupt remapper in all your supported hardware.  That being the case, iommu=force
> 
> Let me take one more shot at this, since no one has yet refuted my
> original points.
> 
> Why do you *need* IR to have a secure Xen w/ TXT?  Certainly a DoS is
> very undesirable, but that is not really a security issue.  Tell me
> what security exploits are still possible with the current patches.

The Invisible Things paper lists a selection of possible attack vectors.  
That they only developed and disclosed one actual exploit is, AIUI, as
much a question of manpower as anything else.  I haven't seen any
analysis from Intel to suggest otherwise.

I think Ian's latest patch is the right thing to do.  But since I'm not
a maintainer of that piece of code, and since in practice the decision
will be made for most people by product and distro engineers anyway, I'm
not going to chase this thread around any more.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI"):
> 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@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Keir, do you think we should apply this then ?

Ian.

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

[Keir Fraser]

On 24/05/2011 16:15, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

> Ian Campbell writes ("Re: [Xen-devel] Xen security advisory CVE-2011-1898 -
> VT-d (PCI passthrough) MSI"):
>> 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@citrix.com>
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> Keir, do you think we should apply this then ?

<sigh> take your pick really. Majority opinion is on the side of this
revised patch, however Intel are the primary maintainers of this code and
they clearly do not like it. If I have a casting vote here, I would be
inclined to plump in favour of the revised patch -- we already have iommu=on
as a best-effort option, and I believe iommu=force could be stronger than it
is. However Joseph's claim that the non-DoS vulns may all now be handled is
not as unconvincing as some seem to believe (and I was in that camp for a
while) -- I can't really see how the attack vector can be successfully
exploited now my mitigation patch is in the tree. So I'm not strongly
inclined one way or the other really.

 -- Keir

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

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Pratt]

> <sigh> take your pick really. Majority opinion is on the side of this
> revised patch, however Intel are the primary maintainers of this code and
> they clearly do not like it. If I have a casting vote here, I would be
> inclined to plump in favour of the revised patch -- we already have
> iommu=on
> as a best-effort option, and I believe iommu=force could be stronger than it
> is. However Joseph's claim that the non-DoS vulns may all now be handled is
> not as unconvincing as some seem to believe (and I was in that camp for a
> while) -- I can't really see how the attack vector can be successfully
> exploited now my mitigation patch is in the tree. So I'm not strongly
> inclined one way or the other really.

My inclination would be such that iommu=force is allowed on non IR systems, but where IR is expected to be present e.g. sandybridge generation we insist that it is enabled (i.e. that the BIOS supports it).

Ian

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Jackson]

Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI"):
> Why do you *need* IR to have a secure Xen w/ TXT?  Certainly a DoS
> is very undesirable, but that is not really a security issue.

I'm afraid that a DoS is very much a security issue.

>  Tell me what security exploits are still possible with the current
> patches.

As I understand it, a DoS (host crash) is still possible.

> If someone can present a security issue that TXT

I don't understand the contribution of TXT to this.  The issue is with
running untrusted guest kernels.  Necessarily an untrusted guest
kernel isn't checked by TXT; that's what "untrusted guest kernel"
means.

Ian.

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Jackson]

Ian Pratt writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI"):
> My inclination would be such that iommu=force is allowed on non IR
> systems, but where IR is expected to be present e.g. sandybridge
> generation we insist that it is enabled (i.e. that the BIOS supports
> it).

I don't think that's a conceptually coherent point of view, unless the
purpose is to avoid marketing embarrassment.

Either IR is required for a secure system with passthrough, in which
case iommu=force should require IR, or it is not required for a secure
system with passthrough, in which case iommu=force should not insist
on it.

Whether it is required for security doesn't depend on whether it is
actually available.  That there are some motherboards which cannot do
passthrough securely does not mean that we should allow users of those
boards to be led up the garden path.

Ian.

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, May 24, 2011 9:57 AM
> 
> Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI
> passthrough) MSI"):
> > Why do you *need* IR to have a secure Xen w/ TXT?  Certainly a DoS is
> > very undesirable, but that is not really a security issue.
> 
> I'm afraid that a DoS is very much a security issue.

Or a reliability/availability issue.  It clearly is not in the same class as security issues that allow for code injection, privilege escalation, etc.  You might even consider this as "fail secure" ;-)

> >  Tell me what security exploits are still possible with the current
> > patches.
> 
> As I understand it, a DoS (host crash) is still possible.

So you would rather cause the DoS as soon as Xen is run (via the panic) instead of if a guest actually tries to use an MSI attack?  How does that make the system more secure?

So if we go back to another point I raised previously...  If you commit your patch, what will be your instructions (to users, etc.) for specifying the 'iommu=' parameters?  I would expect they would be "If your system supports IR then specify 'iommu=force'.  If it supports IOMMU but not IR then specify 'iommu=force,nointremap'." (unless of course you want to say "If it supports IOMMU but not IR then throw it away and buy a new one that supports IR or use KVM instead.").  Of course, the result is the same as the current 'iommu=force' behavior.

> 
> > If someone can present a security issue that TXT
> 
> I don't understand the contribution of TXT to this.  The issue is with running untrusted guest
> kernels.  Necessarily an untrusted guest kernel isn't checked by TXT; that's what "untrusted guest
> kernel"
> means.

TXT does two things:  1) it prevents the SIPI attack (by turning it into a DoS) and 2) it prevents malware from tricking Xen into not enabling IR on a system that supports it.  The second one is what makes the current 'force' behavior the same on an IR system as your patch (i.e. panic/reset).

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, May 24, 2011 10:14 AM
> 
> Ian Pratt writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough)
> MSI"):
> > My inclination would be such that iommu=force is allowed on non IR
> > systems, but where IR is expected to be present e.g. sandybridge
> > generation we insist that it is enabled (i.e. that the BIOS supports
> > it).
> 
> I don't think that's a conceptually coherent point of view, unless the purpose is to avoid
> marketing embarrassment.
> 
> Either IR is required for a secure system with passthrough, in which case iommu=force should
> require IR, or it is not required for a secure system with passthrough, in which case iommu=force
> should not insist on it.

None of the proposed patches check for whether passthrough is being used.  Nor can they check whether it is being used safely (it may be used for performance by domains that are trusted).

Whether IR is required for a secure system with passthrough depends on the usage model (as I indicated in an earlier email).  The user/distributor should decide whether their usage model requires it or not.  If it does, then all they need to do is run on HW that supports IR (and if they're worried about the pre-OS attack then use TXT, which would be necessary anyway).

> Whether it is required for security doesn't depend on whether it is actually available.  That
> there are some motherboards which cannot do passthrough securely does not mean that we should
> allow users of those boards to be led up the garden path.

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI [and 2 more messages]

[Ian Jackson]

Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI"):
> None of the proposed patches check for whether passthrough is being
> used.  Nor can they check whether it is being used safely (it may be
> used for performance by domains that are trusted).

Indeed.  Providing that information is the purpose of the
"iommu=required" command line option.  That option is a declaration by
the system administrator: "I will be using PCI passthrough and I need
it to be secure".

A better arrangement would to prevent the use of PCI passthrough on
insecure systems, at the time of device assignment, unless the
sysadmin has explicitly specified "pci_passthrough_security=none" or
something.  But I imagine you would object to that even more.

> Whether IR is required for a secure system with passthrough depends
> on the usage model (as I indicated in an earlier email).

If the usage model does not depend on Xen enforcing VM separation when
PCI passthrough is used, then "iommu=on" is the correct boot option.
We are only arguing about the behaviour when "iommu=required".

>  The user/distributor should decide whether their usage model
> requires it or not.  If it does, then all they need to do is run on
> HW that supports IR (and if they're worried about the pre-OS attack
> then use TXT, which would be necessary anyway).

If the user has specified "iommu=required" then it is a bug if the
system then allows a domain to be created with pci passthrough devices
in a way that allows the domain to attack the host.

The user should not be required to take additional steps to ensure
that the system is secure.  Your proposal would have the user have to
inspect the boot output from Xen in detail in an effort to determine
whether the hardware and software they had was working properly.
(Determining the hardware feature support from marketing literature or
pre-sales material is obviously not reliable.)

I'm not sure what you mean by "pre-OS attack".  If you mean the
situation with an untrusted guest kernel, that is a very common one
and does not require the use of TXT.  It simply requires Xen to
properly enforce the VM boundary.

Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI"):
> Ian Jackson:
> > I'm afraid that a DoS is very much a security issue.
> 
> Or a reliability/availability issue.  It clearly is not in the same
> class as security issues that allow for code injection, privilege
> escalation, etc.  You might even consider this as "fail secure" ;-)

I don't want to get into an argument about semantics.  Nevertheless,
protection against denial of service is a key requirement for most
software.  In other software projects, when DoS vulnerabilities are
found, they issue a security advisory and fix the bug.  We should do
the same.

> > As I understand it, a DoS (host crash) is still possible.
> 
> So you would rather cause the DoS as soon as Xen is run (via the
> panic) instead of if a guest actually tries to use an MSI attack?
> How does that make the system more secure?

This is getting ridiculous.  I'm really starting to become quite
cross.  Please stop blocking the correct behaviour just because it's
politically inconvenient.

(rest of my draft email snipped)

Ian.

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

[Alan Cox]

> TXT does two things:  1) it prevents the SIPI attack (by turning it into a DoS) and 2) it prevents malware from tricking Xen into not enabling IR on a system that supports it.  The second one is what makes the current 'force' behavior the same on an IR system as your patch (i.e. panic/reset).

1 is a rather curious redefinition of the word "prevents"

It reduces it to taking the system out. That's not "prevent" in my
dictionary.

Alan

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI [and 2 more messages]

[Cihula, Joseph]

> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, May 25, 2011 3:13 AM
> 
> Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI
> passthrough) MSI"):
> > None of the proposed patches check for whether passthrough is being
> > used.  Nor can they check whether it is being used safely (it may be
> > used for performance by domains that are trusted).
> 
> Indeed.  Providing that information is the purpose of the "iommu=required" command line option.
> That option is a declaration by the system administrator: "I will be using PCI passthrough and I
> need it to be secure".
> 
> A better arrangement would to prevent the use of PCI passthrough on insecure systems, at the time
> of device assignment, unless the sysadmin has explicitly specified "pci_passthrough_security=none"
> or something.  But I imagine you would object to that even more.
> 
> > Whether IR is required for a secure system with passthrough depends on
> > the usage model (as I indicated in an earlier email).
> 
> If the usage model does not depend on Xen enforcing VM separation when PCI passthrough is used,
> then "iommu=on" is the correct boot option.
> We are only arguing about the behaviour when "iommu=required".
> 
> >  The user/distributor should decide whether their usage model requires
> > it or not.  If it does, then all they need to do is run on HW that
> > supports IR (and if they're worried about the pre-OS attack then use
> > TXT, which would be necessary anyway).
> 
> If the user has specified "iommu=required" then it is a bug if the system then allows a domain to
> be created with pci passthrough devices in a way that allows the domain to attack the host.
> 
> The user should not be required to take additional steps to ensure that the system is secure.
> Your proposal would have the user have to inspect the boot output from Xen in detail in an effort
> to determine whether the hardware and software they had was working properly.
> (Determining the hardware feature support from marketing literature or pre-sales material is
> obviously not reliable.)
> 
> I'm not sure what you mean by "pre-OS attack".  If you mean the situation with an untrusted guest
> kernel, that is a very common one and does not require the use of TXT.  It simply requires Xen to
> properly enforce the VM boundary.
> 
> Cihula, Joseph writes ("RE: [Xen-devel] Xen security advisory CVE-2011-1898 - VT-d (PCI
> passthrough) MSI"):
> > Ian Jackson:
> > > I'm afraid that a DoS is very much a security issue.
> >
> > Or a reliability/availability issue.  It clearly is not in the same
> > class as security issues that allow for code injection, privilege
> > escalation, etc.  You might even consider this as "fail secure" ;-)
> 
> I don't want to get into an argument about semantics.  Nevertheless, protection against denial of
> service is a key requirement for most software.  In other software projects, when DoS
> vulnerabilities are found, they issue a security advisory and fix the bug.  We should do the same.
> 
> > > As I understand it, a DoS (host crash) is still possible.
> >
> > So you would rather cause the DoS as soon as Xen is run (via the
> > panic) instead of if a guest actually tries to use an MSI attack?
> > How does that make the system more secure?
> 
> This is getting ridiculous.  I'm really starting to become quite cross.  Please stop blocking the
> correct behaviour just because it's politically inconvenient.
> 
> (rest of my draft email snipped)

We'll just have to agree to disagree.  So that said, Ian's (Campbell) last patch should undo the conditional coalescing.

Joe

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

[Ian Campbell]

On Wed, 2011-05-18 at 11:03 +0100, Keir Fraser wrote:
> On 18/05/2011 09:53, "Ian Campbell" <Ian.Campbell@eu.citrix.com> wrote:
> 
> > On Mon, 2011-05-16 at 22:34 +0100, Cihula, Joseph wrote:
> >>> -    if ( iommu_intremap )
> >>> -    {
> >>> +
> >> 
> >> Unless I'm misreading it, this will prevent users from specifying
> >> "no-intremap" to disable the use of IR.
> > 
> > That wasn't my intention.
> > 
> >> Why would you keep the 'if ( iommu_intremap )' on the previous code
> >> block but remove it here?
> > 
> > To be honest this change was a little bit unrelated, which was naughty
> > of me. I saw:
> > if ( iommu_intremap ) {
> > THING A
> > }
> > if ( iommu_intremap ) {
> > THING B
> > }
> > and changed it to
> > if (iommu_intremap ) {
> > THING A
> > THING B
> > }
> > 
> > Is there some subtlety to this code path that I've missed?
> 
> 'THING A' conditionally clears iommu_intremap.

Right, but in this patch I was switching those to panic()s, so maybe not
such an unrelated change after all.

Anyway, I don't think this patch is going to fly so it doesn't really
matter.

Ian.

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

[Keir Fraser]

On 18/05/2011 09:53, "Ian Campbell" <Ian.Campbell@eu.citrix.com> wrote:

> On Mon, 2011-05-16 at 22:34 +0100, Cihula, Joseph wrote:
>>> -    if ( iommu_intremap )
>>> -    {
>>> +
>> 
>> Unless I'm misreading it, this will prevent users from specifying
>> "no-intremap" to disable the use of IR.
> 
> That wasn't my intention.
> 
>> Why would you keep the 'if ( iommu_intremap )' on the previous code
>> block but remove it here?
> 
> To be honest this change was a little bit unrelated, which was naughty
> of me. I saw:
> if ( iommu_intremap ) {
> THING A
> }
> if ( iommu_intremap ) {
> THING B
> }
> and changed it to
> if (iommu_intremap ) {
> THING A
> THING B
> }
> 
> Is there some subtlety to this code path that I've missed?

'THING A' conditionally clears iommu_intremap.

 -- Keir

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

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Campbell]

On Mon, 2011-05-16 at 22:34 +0100, Cihula, Joseph wrote:
> > -    if ( iommu_intremap )
> > -    {
> > +
> 
> Unless I'm misreading it, this will prevent users from specifying
> "no-intremap" to disable the use of IR.

That wasn't my intention.

> Why would you keep the 'if ( iommu_intremap )' on the previous code
> block but remove it here?

To be honest this change was a little bit unrelated, which was naughty
of me. I saw:
	if ( iommu_intremap ) {
		THING A
	}
	if ( iommu_intremap ) {
		THING B
	}
and changed it to 
	if (iommu_intremap ) {
		THING A
		THING B
	}

Is there some subtlety to this code path that I've missed?

Ian.

RE: Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Cihula, Joseph]

[-- Attachment #1: Type: text/plain, Size: 8274 bytes --]

> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On
> Behalf Of Ian Campbell
> Sent: Friday, May 13, 2011 3:25 AM
> 
> On Thu, 2011-05-12 at 14:48 +0100, Ian Jackson wrote:
> > The second patch is intended to ensure that when Xen boots with
> > "iommu=required" it will also insist that interrupt remapping is
> > supported and enabled.  It arranges that booting with that option on
> > vulnerable hardware will fail, rather than appearing to succeed but
> > actually being vulnerable to guests.
> >  Filename: intremap05033.patch
> >  SHA1: 1cd26adc5ead0c07b67bf354f03164235d67395c
> >  SHA256:
> > 7f8c7d95d33bbd5c4f25671b380e70020fda1ba6cb50b67e59131fa8e59c1c66
> 
> This patch became 23338:9751bc49639e but it is not clear that it goes far enough since it appears
> to rely on TXT or similar since it implicitly trusts the contents of DMAR (via the call to
> platform_supports_intremap()).
> 
> I think we should go further and try to be safe by default, even if TXT is not present. (I don't
> actually have a VT-d capable machine handy to properly test this).
> 
> 8<--------------------------------
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com> # Date 1305282251 -3600 # Node ID
> cfffebdedd0b1f1f3f23f60df269f50f7320226b
> # Parent  48d68a57f3e8885366478b418e77b043f73dcb2c
> vt-d: [CVE-2011-1898] refuse to boot with VT-d IOMMU enabled without interupt remapping
> 
> CVE-2011-1898 shows that IOMMU is vulnerable to privilege escalation attacks if Interrupt
> Remapping is not available.
> 
> 23364:9751bc49639e tries to ensure that Interrupt Remapping is always enabled if iommu=required is
> passed on the command line. However this relies on being able to trust the DMAR and therefore
> requires TXT, as well as the user adding that particular option.
> 
> This patch causes the hypervisor to refuse to boot with VT-d IOMMU enabled unless Interrupt
> Remapping is also available. This will prevent unwitting users of older hardware from running in
> an insecure mode when they may think they are secure because they have an IOMMU. It will also
> prevent a malicious guest from tricking a system which does support IOMMU into booting without it.
> 
> Users with pre-Interrupt Remapping hardware who accept the risks are still able to pass iommu=no-
> intremap on the command line in order to revert to previous behaviour.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> diff -r 48d68a57f3e8 -r cfffebdedd0b xen/drivers/passthrough/vtd/iommu.c
> --- a/xen/drivers/passthrough/vtd/iommu.c	Fri May 13 11:10:13 2011 +0100
> +++ b/xen/drivers/passthrough/vtd/iommu.c	Fri May 13 11:24:11 2011 +0100
> @@ -1965,32 +1965,17 @@ static int init_vtd_hw(void)
>          for ( apic = 0; apic < nr_ioapics; apic++ )
>          {
>              if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
> -            {
> -                iommu_intremap = 0;
> -                dprintk(XENLOG_ERR VTDPREFIX,
> -                    "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;
> -            }
> +                panic(VTDPREFIX
> +                      "ioapic_to_iommu: ioapic 0x%x (id: 0x%x) is NULL! "
> +                      "Unable to enable Interrupt Remapping.\n",
> +                      apic, IO_APIC_ID(apic));
>          }
> -    }

I'm OK with this, as the user can always specify "no-intremap" if they really want to boot anyway, and the "force" behavior will be the same.

> -    if ( iommu_intremap )
> -    {
> +

Unless I'm misreading it, this will prevent users from specifying "no-intremap" to disable the use of IR.  Why would you keep the 'if ( iommu_intremap )' on the previous code block but remove it here?

>          for_each_drhd_unit ( drhd )
>          {
>              iommu = drhd->iommu;
>              if ( enable_intremap(iommu, 0) != 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;
> -            }
> +                panic(VTDPREFIX "Failed to enable Interrupt
> + Remapping\n");
>          }
>      }
> 
> @@ -2066,7 +2051,7 @@ int __init intel_vtd_setup(void)
>              iommu_qinval = 0;
> 
>          if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
> -            iommu_intremap = 0;
> +            panic(VTDPREFIX "IOMMU: hardware does not support Interrupt
> + Remapping");
> 
>          if ( !vtd_ept_page_compatible(iommu) )
>              iommu_hap_pt_share = FALSE; @@ -2081,11 +2066,8 @@ int __init intel_vtd_setup(void)
>      }
> 
>      if ( !iommu_qinval && iommu_intremap )
> -    {
> -        iommu_intremap = 0;
> -        dprintk(XENLOG_WARNING VTDPREFIX, "Interrupt Remapping disabled "
> +        panic(XENLOG_WARNING "Interrupt Remapping disabled "
>              "since Queued Invalidation isn't supported or enabled.\n");
> -    }
> 
>  #define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
>      P(iommu_snoop, "Snoop Control");


I disagree with these parts as well.

These changes would say that if the user wants IOMMU enabled that the system must also support IR.  Let me list several issues with such an approach:

This may actually weaken security rather than increase it.  Let's face it, most users won't buy new HW just because the latest version of their SW breaks on their current HW (were it only so easy ;-) ).  So those users will be forced to either turn off IOMMU or not upgrade Xen.  In the former case, they will lose the ability to separate the hypervisor from domains (even dom0 is not equally privileged to the hypervisor) as well as having decreased robustness from driver bugs/FW bugs/etc.  In the latter case, they will not get any of the other security and feature fixes from newer versions of Xen.

MSI support has been on systems before there was even an IOMMU capability.  Why does adding IOMMU make these systems suddenly insecure?  All of the same uses that existed before IOMMU are still valid even with it--why disallow them (see above as to why forcing user to turn off IOMMU is bad).

IOMMU adds security capabilities.  IR adds additional security capabilities.  IOMMU allows for fully isolating the hypervisor from domains even if the domains control DMA devices.  It helps to protect against buggy drivers or device FW by limiting the areas such bugs can affect to just the DMA data buffers.  IOMMU, in conjunction with Intel(R) Trusted Execution Technology (TXT), provides DMA protection through the entire launch process and into runtime.  This is all true regardless of the presence of IR.  IR adds the ability to prevent DoS attacks by untrusted domains with assigned DMA devices, malicious device FW, etc.  This is incremental--not all or nothing.

The 00-block-msis-on-trap-vectors patch (esp. in conjunction with TXT) prevents all known security exploits of MSI misuse.  Might there still be vulnerabilities to MSI--yes, and when they're found they will be fixed.  If we find a code injection bug that would have been prevented by XD/NX, do we disallow running on systems without XD/NX capability just because there might be more such bugs--no, we fix the ones we know about and will fix others as they are found.

If you really want to fail to run insecurely, here is the patch you need:
diff -r f9bb0bbea7c2 xen/arch/x86/boot/head.S
--- a/xen/arch/x86/boot/head.S  Thu May 12 16:42:54 2011 +0100
+++ b/xen/arch/x86/boot/head.S  Mon May 16 14:40:50 2011 -0700
@@ -20,7 +20,7 @@
 #define BOOT_PSEUDORM_DS 0x0028

 ENTRY(start)
-        jmp     __start
+        hlt

         .align 4
 /*** MULTIBOOT HEADER ****/

Joe

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Joanna Rutkowska]

[-- Attachment #1.1: Type: text/plain, Size: 394 bytes --]

On 05/13/11 19:32, Joanna Rutkowska wrote:
> Our paper describing the attacks can be now downloaded from here:
> 
> http://www.invisiblethingslab.com/itl/Resources.html
> 
> (Sorry the actual link contains spaces and would likely by unclickable
> if I pasted it here).
> 
or perhaps not:

http://www.invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 518 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Joanna Rutkowska]

[-- Attachment #1.1: Type: text/plain, Size: 4914 bytes --]

Our paper describing the attacks can be now downloaded from here:

http://www.invisiblethingslab.com/itl/Resources.html

(Sorry the actual link contains spaces and would likely by unclickable
if I pasted it here).

Cheers,
joanna.

On 05/12/11 15:48, Ian Jackson wrote:
>              Xen security advisory CVE-2011-1898
>            VT-d (PCI passthrough) MSI trap injection
> 
> ISSUE DESCRIPTION
> =================
> 
> Intel VT-d chipsets without interrupt remapping do not prevent a guest
> which owns a PCI device from using DMA to generate MSI interrupts by
> writing to the interrupt injection registers.  This can be exploited
> to inject traps and gain control of the host.
> 
> 
> VULNERABLE SYSTEMS
> ==================
> 
> You are not vulnerable if you do not use "PCI passthrough".  That is,
> if you do not pass actual PCI devices (eg, graphics and network
> controllers) through to guests, for use by PCI device drivers in the
> guest.
> 
> In Xen with xend/xm or with xl this would be enabled by the "pci="
> option in the domain config file, or by using the "xl pci-attach" or
> "xm pci-attach" management command; if you do not use these features,
> you are not vulnerable.
> 
> You are not vulnerable if you are using PCI passthrough, but are not
> using Intel VT-d or AMD-Vi (aka "iommu") to attempt to prevent escape
> by guest DMA.  This is because in such a configuration, privilege
> escalation and denial of service are possible by guests anyway, and
> the present issue does not make the situation any worse.
> 
> You are vulnerable if you use Intel VT-d to pass PCI devices through
> to untrusted guests, unless your have interrupt remapping supported
> and enabled.  This is the case whether you are using Xen, KVM, or
> another virtualisation system.
> 
> Interrupt remapping is available in newer Intel VT-d chipsets.
> 
> 
> IMPACT
> ======
> 
> A guest given a PCI passthrough device can escalate its privilege and
> gain control of the whole system.
> 
> 
> MITIGATION AND RESOLUTION
> =========================
> 
> No complete software fix is available but we understand that Intel has
> addressed this issue with newer hardware.
> 
> We believe a patch along the lines of the one attached can be applied
> to Xen to reduce the impact to a denial of service.  Even with such a
> patch, a guest can still cause a complete system crash or resource
> starvation.
> 
> Upgrading to recent hardware that is interrupt remapping capable will
> resolve the remaining denial of service issues.  Support for interrupt
> remapping, when the hardware is capable, is present in all currently
> maintained versions of Xen.
> 
> On such recent hardware, when passing pci devices through to untrusted
> guests, we recommend the use of the "iommu=required" Xen command line
> boot option and the second atttached patch, to avoid unknowingly
> booting into a vulnerable configuration.
> 
> 
> REFERENCES
> ==========
> 
> Thanks to Rafal Wojtczuk and Joanna Rutkowska of Invisible Things Lab
> for bringing this issue to our attention.  Their paper on the attack
> will soon be available from Invisible Things Lab, at
> www.invisiblethingslab.com.
> 
> Information regarding chipset versions and interrupt remapping support
> should be available from Intel; please use your usual support and
> security response channels at Intel.
> 
> We believe that this vulnerability exists with all virtualisation
> systems which aim to support passing pci devices through to
> untrusted guests, on the affected Intel hardware.  If you are using
> a hypervisor other than Xen please refer to your hypervisor's usual
> security support and advisory release channels.
> 
> 
> PATCHES
> =======
> 
> The first patch is intended to reduce the impact from full privilege
> escalation to denial of service.
>  Filename: 00-block-msis-on-trap-vectors
>  SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
>  SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9
> 
> The second patch is intended to ensure that when Xen boots with
> "iommu=required" it will also insist that interrupt remapping is
> supported and enabled.  It arranges that booting with that option on
> vulnerable hardware will fail, rather than appearing to succeed but
> actually being vulnerable to guests.
>  Filename: intremap05033.patch
>  SHA1: 1cd26adc5ead0c07b67bf354f03164235d67395c
>  SHA256: 7f8c7d95d33bbd5c4f25671b380e70020fda1ba6cb50b67e59131fa8e59c1c66
> 
> Unfortunately we have not been able to test either patch.  Both will
> be applied to xen-unstable very soon.  We also intend to provide
> backports in the supported released Xen trees.
> 

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



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 518 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Tim Deegan]

At 13:29 +0100 on 13 May (1305293351), Jan Beulich wrote:
> So are you saying that the memory transaction triggering the MSI is
> indistinguishable from any other DMA operation? Implying that the
> guest must be granted access to the page containing the MSI
> address the device is to write to? If so, the changes done as a
> result of your report are only addressing a (very?) small subset of
> bad things such a guest could do.

Yes, and yes.  The only real fix is for the hardware to do interrupt
remapping, and the hypervisor to enforce it.  The patches that go with
the advisory only reduce a full exploit to a DoS (and so, whether you
kill all device-owning domains or the whole hypervisor is pretty much
moot).

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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

[Jan Beulich]

>>> On 13.05.11 at 13:20, Joanna Rutkowska <joanna@invisiblethingslab.com> wrote:
> On 05/13/11 13:11, Ian Campbell wrote:
>> On Fri, 2011-05-13 at 12:08 +0100, Joanna Rutkowska wrote:
>>> On 05/13/11 10:08, Jan Beulich wrote:
>> 
>>>> Finally, wouldn't killing all guests that potentially could have caused
>>>> the problem be a better measure than bringing down the host?
>>>>
>>>
>>> Killing the guest might no longer be enough, because the guest might
>>> have already programmed the device to keep sending malicious MSIs.
>> 
>> Is it even possible to know which guest triggered the MSI, or is the
>> best you can do the set of all guests with an MSI capable device passed
>> through?
>> 
> 
> Ah, probably you're right -- if we have more than one driver domain,
> then I think LAPIC would not tell us which device genrated the MSI.

That's why I wrote "killing all guests that potentially could have ...".

> In fact it's not really correct to assume that it must have been a guest
> with a "MSI capable device" -- note that we don't trigger the MSI via
> the official MSI triggering mechanism.

You lost me here. Neither am I clear about what "non-official"
triggering mechanism we use, nor can I see how a guest without
any MSI-capable device would be able to trigger the problem.

And even if things are as you say, it would still seem better to kill
all guests with *any* passed through device, than bring down the
entire host (there could e.g. be dozens of innocent pv guests and
only a single hvm one that has a problematic device assigned).

Jan

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

[Jan Beulich]

>>> On 13.05.11 at 13:08, Joanna Rutkowska <joanna@invisiblethingslab.com> wrote:
> On 05/13/11 10:08, Jan Beulich wrote:
>>>>> On 12.05.11 at 15:48, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>>> Intel VT-d chipsets without interrupt remapping do not prevent a guest
>>> which owns a PCI device from using DMA to generate MSI interrupts by
>>> writing to the interrupt injection registers.  This can be exploited
>>> to inject traps and gain control of the host.
>> 
>> Isn't that (or at least can't that be) prevented with DMA remapping?
>> 
> 
> No. That's sort of the key point here, and the reason why IR hardware is
> required.

So are you saying that the memory transaction triggering the MSI is
indistinguishable from any other DMA operation? Implying that the
guest must be granted access to the page containing the MSI
address the device is to write to? If so, the changes done as a
result of your report are only addressing a (very?) small subset of
bad things such a guest could do.

>>> The first patch is intended to reduce the impact from full privilege
>>> escalation to denial of service.
>>>  Filename: 00-block-msis-on-trap-vectors
>>>  SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
>>>  SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9
>> 
>> You modify only 64-bit and only VT-d code here. While I know you
>> don't care much for it, doing the same for 32-bit would seem trivial.
>> 
>> As to AMD's IOMMU, it may well be that interrupt re-mapping isn't
>> optional in the hardware (albeit it can be disabled on the command
>> line, though that's the admin's security risk then), but the code
>> having BUG_ON()s on failed allocations and those allocations
>> happening in table parsing callbacks doesn't really make this
>> explicit (for me at least) on the first glance.
>> 
>> Finally, wouldn't killing all guests that potentially could have caused
>> the problem be a better measure than bringing down the host?
>> 
> 
> Killing the guest might no longer be enough, because the guest might
> have already programmed the device to keep sending malicious MSIs. So,

Obviously, resetting the devices should be part of killing such guests.

Jan

> panic()ing the whole VMM seems like a safer choice. Keep in mind that on
> a non-IR hardware there are probably many other ways for the malicious
> driver domain to cause global DoS. (In fact, my impression is that most
> people regarded IR as an anti-DoS mechanism, and I believe our paper is
> the first to show that the problems were far worse than possible DoS.)
> 
> joanna.

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

[Joanna Rutkowska]

[-- Attachment #1.1: Type: text/plain, Size: 952 bytes --]

On 05/13/11 13:11, Ian Campbell wrote:
> On Fri, 2011-05-13 at 12:08 +0100, Joanna Rutkowska wrote:
>> On 05/13/11 10:08, Jan Beulich wrote:
> 
>>> Finally, wouldn't killing all guests that potentially could have caused
>>> the problem be a better measure than bringing down the host?
>>>
>>
>> Killing the guest might no longer be enough, because the guest might
>> have already programmed the device to keep sending malicious MSIs.
> 
> Is it even possible to know which guest triggered the MSI, or is the
> best you can do the set of all guests with an MSI capable device passed
> through?
> 

Ah, probably you're right -- if we have more than one driver domain,
then I think LAPIC would not tell us which device genrated the MSI.

In fact it's not really correct to assume that it must have been a guest
with a "MSI capable device" -- note that we don't trigger the MSI via
the official MSI triggering mechanism.

joanna.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 518 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Ian Campbell]

On Fri, 2011-05-13 at 12:08 +0100, Joanna Rutkowska wrote:
> On 05/13/11 10:08, Jan Beulich wrote:

> > Finally, wouldn't killing all guests that potentially could have caused
> > the problem be a better measure than bringing down the host?
> > 
> 
> Killing the guest might no longer be enough, because the guest might
> have already programmed the device to keep sending malicious MSIs.

Is it even possible to know which guest triggered the MSI, or is the
best you can do the set of all guests with an MSI capable device passed
through?

Ian.

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

[Joanna Rutkowska]

[-- Attachment #1.1: Type: text/plain, Size: 2213 bytes --]

On 05/13/11 10:08, Jan Beulich wrote:
>>>> On 12.05.11 at 15:48, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> Intel VT-d chipsets without interrupt remapping do not prevent a guest
>> which owns a PCI device from using DMA to generate MSI interrupts by
>> writing to the interrupt injection registers.  This can be exploited
>> to inject traps and gain control of the host.
> 
> Isn't that (or at least can't that be) prevented with DMA remapping?
> 

No. That's sort of the key point here, and the reason why IR hardware is
required.

>> The first patch is intended to reduce the impact from full privilege
>> escalation to denial of service.
>>  Filename: 00-block-msis-on-trap-vectors
>>  SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
>>  SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9
> 
> You modify only 64-bit and only VT-d code here. While I know you
> don't care much for it, doing the same for 32-bit would seem trivial.
> 
> As to AMD's IOMMU, it may well be that interrupt re-mapping isn't
> optional in the hardware (albeit it can be disabled on the command
> line, though that's the admin's security risk then), but the code
> having BUG_ON()s on failed allocations and those allocations
> happening in table parsing callbacks doesn't really make this
> explicit (for me at least) on the first glance.
> 
> Finally, wouldn't killing all guests that potentially could have caused
> the problem be a better measure than bringing down the host?
> 

Killing the guest might no longer be enough, because the guest might
have already programmed the device to keep sending malicious MSIs. So,
panic()ing the whole VMM seems like a safer choice. Keep in mind that on
a non-IR hardware there are probably many other ways for the malicious
driver domain to cause global DoS. (In fact, my impression is that most
people regarded IR as an anti-DoS mechanism, and I believe our paper is
the first to show that the problems were far worse than possible DoS.)

joanna.

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



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 518 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

[Ian Campbell]

On Thu, 2011-05-12 at 14:48 +0100, Ian Jackson wrote:
> The second patch is intended to ensure that when Xen boots with
> "iommu=required" it will also insist that interrupt remapping is
> supported and enabled.  It arranges that booting with that option on
> vulnerable hardware will fail, rather than appearing to succeed but
> actually being vulnerable to guests.
>  Filename: intremap05033.patch
>  SHA1: 1cd26adc5ead0c07b67bf354f03164235d67395c
>  SHA256:
> 7f8c7d95d33bbd5c4f25671b380e70020fda1ba6cb50b67e59131fa8e59c1c66

This patch became 23338:9751bc49639e but it is not clear that it goes
far enough since it appears to rely on TXT or similar since it
implicitly trusts the contents of DMAR (via the call to
platform_supports_intremap()).

I think we should go further and try to be safe by default, even if TXT
is not present. (I don't actually have a VT-d capable machine handy to
properly test this).

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

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1305282251 -3600
# Node ID cfffebdedd0b1f1f3f23f60df269f50f7320226b
# Parent  48d68a57f3e8885366478b418e77b043f73dcb2c
vt-d: [CVE-2011-1898] refuse to boot with VT-d IOMMU enabled without interupt remapping

CVE-2011-1898 shows that IOMMU is vulnerable to privilege escalation attacks if
Interrupt Remapping is not available.

23364:9751bc49639e tries to ensure that Interrupt Remapping is always enabled
if iommu=required is passed on the command line. However this relies on being
able to trust the DMAR and therefore requires TXT, as well as the user adding
that particular option.

This patch causes the hypervisor to refuse to boot with VT-d IOMMU enabled
unless Interrupt Remapping is also available. This will prevent unwitting users
of older hardware from running in an insecure mode when they may think they are
secure because they have an IOMMU. It will also prevent a malicious guest from
tricking a system which does support IOMMU into booting without it.

Users with pre-Interrupt Remapping hardware who accept the risks are still able to
pass iommu=no-intremap on the command line in order to revert to previous
behaviour.

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

diff -r 48d68a57f3e8 -r cfffebdedd0b xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Fri May 13 11:10:13 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Fri May 13 11:24:11 2011 +0100
@@ -1965,32 +1965,17 @@ static int init_vtd_hw(void)
         for ( apic = 0; apic < nr_ioapics; apic++ )
         {
             if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
-            {
-                iommu_intremap = 0;
-                dprintk(XENLOG_ERR VTDPREFIX,
-                    "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;
-            }
+                panic(VTDPREFIX
+                      "ioapic_to_iommu: ioapic 0x%x (id: 0x%x) is NULL! "
+                      "Unable to enable Interrupt Remapping.\n",
+                      apic, IO_APIC_ID(apic));
         }
-    }
-    if ( iommu_intremap )
-    {
+
         for_each_drhd_unit ( drhd )
         {
             iommu = drhd->iommu;
             if ( enable_intremap(iommu, 0) != 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;
-            }
+                panic(VTDPREFIX "Failed to enable Interrupt Remapping\n");
         }
     }
 
@@ -2066,7 +2051,7 @@ int __init intel_vtd_setup(void)
             iommu_qinval = 0;
 
         if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
-            iommu_intremap = 0;
+            panic(VTDPREFIX "IOMMU: hardware does not support Interrupt Remapping");
 
         if ( !vtd_ept_page_compatible(iommu) )
             iommu_hap_pt_share = FALSE;
@@ -2081,11 +2066,8 @@ int __init intel_vtd_setup(void)
     }
 
     if ( !iommu_qinval && iommu_intremap )
-    {
-        iommu_intremap = 0;
-        dprintk(XENLOG_WARNING VTDPREFIX, "Interrupt Remapping disabled "
+        panic(XENLOG_WARNING "Interrupt Remapping disabled "
             "since Queued Invalidation isn't supported or enabled.\n");
-    }
 
 #define P(p,s) printk("Intel VT-d %s %senabled.\n", s, (p)? "" : "not ")
     P(iommu_snoop, "Snoop Control");

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

[Jan Beulich]

>>> On 12.05.11 at 15:48, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Intel VT-d chipsets without interrupt remapping do not prevent a guest
> which owns a PCI device from using DMA to generate MSI interrupts by
> writing to the interrupt injection registers.  This can be exploited
> to inject traps and gain control of the host.

Isn't that (or at least can't that be) prevented with DMA remapping?

> The first patch is intended to reduce the impact from full privilege
> escalation to denial of service.
>  Filename: 00-block-msis-on-trap-vectors
>  SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
>  SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9

You modify only 64-bit and only VT-d code here. While I know you
don't care much for it, doing the same for 32-bit would seem trivial.

As to AMD's IOMMU, it may well be that interrupt re-mapping isn't
optional in the hardware (albeit it can be disabled on the command
line, though that's the admin's security risk then), but the code
having BUG_ON()s on failed allocations and those allocations
happening in table parsing callbacks doesn't really make this
explicit (for me at least) on the first glance.

Finally, wouldn't killing all guests that potentially could have caused
the problem be a better measure than bringing down the host?

Jan

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

[Ian Jackson]

I wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
>              Xen security advisory CVE-2011-1898
>            VT-d (PCI passthrough) MSI trap injection
...
> The first patch is intended to reduce the impact from full privilege
> escalation to denial of service.
>  Filename: 00-block-msis-on-trap-vectors
>  SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
>  SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9
> 
> The second patch is intended to ensure that when Xen boots with
> "iommu=required" it will also insist that interrupt remapping is
> supported and enabled.  It arranges that booting with that option on
> vulnerable hardware will fail, rather than appearing to succeed but
> actually being vulnerable to guests.
>  Filename: intremap05033.patch
>  SHA1: 1cd26adc5ead0c07b67bf354f03164235d67395c
>  SHA256: 7f8c7d95d33bbd5c4f25671b380e70020fda1ba6cb50b67e59131fa8e59c1c66

These patches should probably be applied to xen-unstable now.

Ian.

Xen security advisory CVE-2011-1898 - VT-d (PCI passthrough) MSI

[Ian Jackson]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 4521 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

             Xen security advisory CVE-2011-1898
           VT-d (PCI passthrough) MSI trap injection

ISSUE DESCRIPTION
=================

Intel VT-d chipsets without interrupt remapping do not prevent a guest
which owns a PCI device from using DMA to generate MSI interrupts by
writing to the interrupt injection registers.  This can be exploited
to inject traps and gain control of the host.


VULNERABLE SYSTEMS
==================

You are not vulnerable if you do not use "PCI passthrough".  That is,
if you do not pass actual PCI devices (eg, graphics and network
controllers) through to guests, for use by PCI device drivers in the
guest.

In Xen with xend/xm or with xl this would be enabled by the "pci="
option in the domain config file, or by using the "xl pci-attach" or
"xm pci-attach" management command; if you do not use these features,
you are not vulnerable.

You are not vulnerable if you are using PCI passthrough, but are not
using Intel VT-d or AMD-Vi (aka "iommu") to attempt to prevent escape
by guest DMA.  This is because in such a configuration, privilege
escalation and denial of service are possible by guests anyway, and
the present issue does not make the situation any worse.

You are vulnerable if you use Intel VT-d to pass PCI devices through
to untrusted guests, unless your have interrupt remapping supported
and enabled.  This is the case whether you are using Xen, KVM, or
another virtualisation system.

Interrupt remapping is available in newer Intel VT-d chipsets.


IMPACT
======

A guest given a PCI passthrough device can escalate its privilege and
gain control of the whole system.


MITIGATION AND RESOLUTION
=========================

No complete software fix is available but we understand that Intel has
addressed this issue with newer hardware.

We believe a patch along the lines of the one attached can be applied
to Xen to reduce the impact to a denial of service.  Even with such a
patch, a guest can still cause a complete system crash or resource
starvation.

Upgrading to recent hardware that is interrupt remapping capable will
resolve the remaining denial of service issues.  Support for interrupt
remapping, when the hardware is capable, is present in all currently
maintained versions of Xen.

On such recent hardware, when passing pci devices through to untrusted
guests, we recommend the use of the "iommu=required" Xen command line
boot option and the second atttached patch, to avoid unknowingly
booting into a vulnerable configuration.


REFERENCES
==========

Thanks to Rafal Wojtczuk and Joanna Rutkowska of Invisible Things Lab
for bringing this issue to our attention.  Their paper on the attack
will soon be available from Invisible Things Lab, at
www.invisiblethingslab.com.

Information regarding chipset versions and interrupt remapping support
should be available from Intel; please use your usual support and
security response channels at Intel.

We believe that this vulnerability exists with all virtualisation
systems which aim to support passing pci devices through to
untrusted guests, on the affected Intel hardware.  If you are using
a hypervisor other than Xen please refer to your hypervisor's usual
security support and advisory release channels.


PATCHES
=======

The first patch is intended to reduce the impact from full privilege
escalation to denial of service.
 Filename: 00-block-msis-on-trap-vectors
 SHA1: 0fcc1914714c228e98b3e84597e06cb5de09003c
 SHA256: 998e8d5632ee6ad92f52796fe94923f9c38096c5adf2ca74209a6792436ea1e9

The second patch is intended to ensure that when Xen boots with
"iommu=required" it will also insist that interrupt remapping is
supported and enabled.  It arranges that booting with that option on
vulnerable hardware will fail, rather than appearing to succeed but
actually being vulnerable to guests.
 Filename: intremap05033.patch
 SHA1: 1cd26adc5ead0c07b67bf354f03164235d67395c
 SHA256: 7f8c7d95d33bbd5c4f25671b380e70020fda1ba6cb50b67e59131fa8e59c1c66

Unfortunately we have not been able to test either patch.  Both will
be applied to xen-unstable very soon.  We also intend to provide
backports in the supported released Xen trees.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iJwEAQECAAYFAk3L5JUACgkQQz2i6ICVfYMh9wP/S/D3xN48bKBM2sBNuO08Hqhy
0ViFGdgZGs3p5EbRCIRTGe6YZonh8oIJi/JSxevd5WD7gDeV+8Dfr4in/gKXWThI
6kyJCzMZdJeB4ecJsX64sKS9pShJT39J1RAoeLeEGiPahO1Id6UySyF23xoF0R0E
7RpPPfG5lxS0xwdQO4M=
=yfWW
-----END PGP SIGNATURE-----


[-- Attachment #2: mitigation patch for old hardware --]
[-- Type: application/octet-stream, Size: 4213 bytes --]

# HG changeset patch
# User Keir Fraser <keir@xen.org>
# Date 1304332766 -3600
# Node ID 0bc9f306cb5222f39b0edcc830fb75d796ef146e
# Parent  24346f749826275540ab7bc95980355ec96c8cf8
x86, vtd: Protect against malicious MSIs from untrusted devices.

In the absence of VT-d interrupt remapping support, a device can send
arbitrary APIC messages to host CPUs. One class of attack that results
is to confuse the hypervisor by delivering asynchronous interrupts to
vectors that are expected to handle only synchronous
traps/exceptions.

We block this class of attack by:
(1) setting APIC.TPR=0x10, to block all interrupts below vector
0x20. This blocks delivery to all architectural exception vectors.
(2) checking APIC.ISR[vec] for vectors 0x80 (fast syscall) and 0x82
(hypercall). In these cases we BUG if we detect we are handling a
hardware interrupt -- turning a potentially more severe infiltration
into a straightforward system crash (i.e, DoS).

Thanks to Invisible Things Lab <http://www.invisiblethingslab.com>
for discovery and detailed investigation of this attack.

Signed-off-by: Keir Fraser <keir@xen.org>

diff -r 24346f749826 -r 0bc9f306cb52 xen/arch/x86/apic.c
--- a/xen/arch/x86/apic.c	Sun May 01 13:17:44 2011 +0100
+++ b/xen/arch/x86/apic.c	Mon May 02 11:39:26 2011 +0100
@@ -560,12 +560,9 @@
     init_apic_ldr();
 
     /*
-     * Set Task Priority to 'accept all'. We never change this
-     * later on.
+     * Set Task Priority to reject any interrupts below FIRST_DYNAMIC_VECTOR.
      */
-    value = apic_read(APIC_TASKPRI);
-    value &= ~APIC_TPRI_MASK;
-    apic_write_around(APIC_TASKPRI, value);
+    apic_write_around(APIC_TASKPRI, (FIRST_DYNAMIC_VECTOR & 0xF0) - 0x10);
 
     /*
      * After a crash, we no longer service the interrupts and a pending
@@ -1439,3 +1436,9 @@
 
     return 0;
 }
+
+void check_for_unexpected_msi(unsigned int vector)
+{
+    unsigned long v = apic_read(APIC_ISR + ((vector & ~0x1f) >> 1));
+    BUG_ON(v & (1 << (vector & 0x1f)));
+}
diff -r 24346f749826 -r 0bc9f306cb52 xen/arch/x86/x86_64/compat/entry.S
--- a/xen/arch/x86/x86_64/compat/entry.S	Sun May 01 13:17:44 2011 +0100
+++ b/xen/arch/x86/x86_64/compat/entry.S	Mon May 02 11:39:26 2011 +0100
@@ -10,12 +10,22 @@
 #include <asm/page.h>
 #include <asm/desc.h>
 #include <public/xen.h>
+#include <irq_vectors.h>
 
         ALIGN
 ENTRY(compat_hypercall)
         pushq $0
         movl  $TRAP_syscall,4(%rsp)
         SAVE_ALL
+
+        cmpb  $0,untrusted_msi(%rip)
+UNLIKELY_START(ne, msi_check)
+        movl  $HYPERCALL_VECTOR,%edi
+        call  check_for_unexpected_msi
+        RESTORE_ALL
+        SAVE_ALL
+UNLIKELY_END(msi_check)
+
         GET_CURRENT(%rbx)
 
         cmpl  $NR_hypercalls,%eax
diff -r 24346f749826 -r 0bc9f306cb52 xen/arch/x86/x86_64/entry.S
--- a/xen/arch/x86/x86_64/entry.S	Sun May 01 13:17:44 2011 +0100
+++ b/xen/arch/x86/x86_64/entry.S	Mon May 02 11:39:26 2011 +0100
@@ -297,6 +297,14 @@
         pushq $0
         SAVE_ALL
 
+        cmpb  $0,untrusted_msi(%rip)
+UNLIKELY_START(ne, msi_check)
+        movl  $0x80,%edi
+        call  check_for_unexpected_msi
+        RESTORE_ALL
+        SAVE_ALL
+UNLIKELY_END(msi_check)
+
         GET_CURRENT(%rbx)
 
         /* Check that the callback is non-null. */
diff -r 24346f749826 -r 0bc9f306cb52 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Sun May 01 13:17:44 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Mon May 02 11:39:26 2011 +0100
@@ -45,6 +45,9 @@
 #define nr_ioapics              iosapic_get_nr_iosapics()
 #endif
 
+/* Possible unfiltered LAPIC/MSI messages from untrusted sources? */
+bool_t __read_mostly untrusted_msi;
+
 int nr_iommus;
 
 static void setup_dom0_devices(struct domain *d);
@@ -1579,6 +1582,14 @@
     if (!pdev)
         return -ENODEV;
 
+    /*
+     * Devices assigned to untrusted domains (here assumed to be any domU)
+     * can attempt to send arbitrary LAPIC/MSI messages. We are unprotected
+     * by the root complex unless interrupt remapping is enabled.
+     */
+    if ( (target != dom0) && !iommu_intremap )
+        untrusted_msi = 1;
+
     ret = domain_context_unmap(source, bus, devfn);
     if ( ret )
         return ret;

[-- Attachment #3: assurance patch for new hardware --]
[-- Type: application/octet-stream, Size: 1035 bytes --]

diff -r 2f08c89b767d xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed Apr 20 17:13:08 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Tue May 03 16:54:24 2011 -0700
@@ -1960,6 +1960,8 @@ 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;
             }
         }
@@ -1973,6 +1975,9 @@ static int init_vtd_hw(void)
             {
                 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;
             }
         }

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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