linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: tip-bot for Andy Lutomirski <luto@amacapital.net>
To: linux-tip-commits@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, hpa@zytor.com, mingo@kernel.org,
	alex.williamson@redhat.com, luto@amacapital.net, gleb@redhat.com,
	suresh.b.siddha@intel.com, tglx@linutronix.de, prarit@redhat.com,
	dzickus@redhat.com
Subject: [tip:x86/apic] x86/intel/irq_remapping: Clean up x2apic opt-out security warning mess
Date: Sun, 3 Feb 2013 11:29:38 -0800	[thread overview]
Message-ID: <tip-af8d102f999a41c0189bd2cce488bac2ee88c29b@git.kernel.org> (raw)
In-Reply-To: <2011b943a886fd7c46079eb10bc24fc130587503.1359759303.git.luto@amacapital.net>

Commit-ID:  af8d102f999a41c0189bd2cce488bac2ee88c29b
Gitweb:     http://git.kernel.org/tip/af8d102f999a41c0189bd2cce488bac2ee88c29b
Author:     Andy Lutomirski <luto@amacapital.net>
AuthorDate: Fri, 1 Feb 2013 14:57:43 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 3 Feb 2013 12:13:48 +0100

x86/intel/irq_remapping: Clean up x2apic opt-out security warning mess

Current kernels print this on my Dell server:

   ------------[ cut here ]------------
   WARNING: at drivers/iommu/intel_irq_remapping.c:542
   intel_enable_irq_remapping+0x7b/0x27e()
   Hardware name: PowerEdge R620
   Your BIOS is broken and requested that x2apic be disabled
   This will leave your machine vulnerable to irq-injection attacks
   Use 'intremap=no_x2apic_optout' to override BIOS request
   [...]
   Enabled IRQ remapping in xapic mode
   x2apic not enabled, IRQ remapping is in xapic mode

This is inconsistent with itself -- interrupt remapping is *on*.

Fix the mess by making the warnings say what they mean and my
making sure that compatibility format interrupts (the dangerous
ones) are disabled if x2apic is present regardless of BIOS
settings.

With this patch applied, the output is:

  Your BIOS is broken and requested that x2apic be disabled.
  This will slightly decrease performance.
  Use 'intremap=no_x2apic_optout' to override BIOS request.
  Enabled IRQ remapping in xapic mode
  x2apic not enabled, IRQ remapping is in xapic mode

This should make us as or more secure than we are now and
replace a rather scary warning with a much less scary warning on
silly but functional systems.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Link: http://lkml.kernel.org/r/2011b943a886fd7c46079eb10bc24fc130587503.1359759303.git.luto@amacapital.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/iommu/intel_irq_remapping.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 7ca4947..f3b8f23 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -429,11 +429,22 @@ static void iommu_set_irq_remapping(struct intel_iommu *iommu, int mode)
 
 	/* Enable interrupt-remapping */
 	iommu->gcmd |= DMA_GCMD_IRE;
+	iommu->gcmd &= ~DMA_GCMD_CFI;  /* Block compatibility-format MSIs */
 	writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
 
 	IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG,
 		      readl, (sts & DMA_GSTS_IRES), sts);
 
+	/*
+	 * With CFI clear in the Global Command register, we should be
+	 * protected from dangerous (i.e. compatibility) interrupts
+	 * regardless of x2apic status.  Check just to be sure.
+	 */
+	if (sts & DMA_GSTS_CFIS)
+		WARN(1, KERN_WARNING
+			"Compatibility-format IRQs enabled despite intr remapping;\n"
+			"you are vulnerable to IRQ injection.\n");
+
 	raw_spin_unlock_irqrestore(&iommu->register_lock, flags);
 }
 
@@ -530,20 +541,24 @@ static int __init intel_irq_remapping_supported(void)
 static int __init intel_enable_irq_remapping(void)
 {
 	struct dmar_drhd_unit *drhd;
+	bool x2apic_present;
 	int setup = 0;
 	int eim = 0;
 
+	x2apic_present = x2apic_supported();
+
 	if (parse_ioapics_under_ir() != 1) {
 		printk(KERN_INFO "Not enable interrupt remapping\n");
-		return -1;
+		goto error;
 	}
 
-	if (x2apic_supported()) {
+	if (x2apic_present) {
 		eim = !dmar_x2apic_optout();
-		WARN(!eim, KERN_WARNING
-			   "Your BIOS is broken and requested that x2apic be disabled\n"
-			   "This will leave your machine vulnerable to irq-injection attacks\n"
-			   "Use 'intremap=no_x2apic_optout' to override BIOS request\n");
+		if (!eim)
+			printk(KERN_WARNING
+				"Your BIOS is broken and requested that x2apic be disabled.\n"
+				"This will slightly decrease performance.\n"
+				"Use 'intremap=no_x2apic_optout' to override BIOS request.\n");
 	}
 
 	for_each_drhd_unit(drhd) {
@@ -582,7 +597,7 @@ static int __init intel_enable_irq_remapping(void)
 		if (eim && !ecap_eim_support(iommu->ecap)) {
 			printk(KERN_INFO "DRHD %Lx: EIM not supported by DRHD, "
 			       " ecap %Lx\n", drhd->reg_base_addr, iommu->ecap);
-			return -1;
+			goto error;
 		}
 	}
 
@@ -598,7 +613,7 @@ static int __init intel_enable_irq_remapping(void)
 			printk(KERN_ERR "DRHD %Lx: failed to enable queued, "
 			       " invalidation, ecap %Lx, ret %d\n",
 			       drhd->reg_base_addr, iommu->ecap, ret);
-			return -1;
+			goto error;
 		}
 	}
 
@@ -637,6 +652,11 @@ error:
 	/*
 	 * handle error condition gracefully here!
 	 */
+
+	if (x2apic_present)
+		WARN(1, KERN_WARNING
+			"Failed to enable irq remapping.  You are vulnerable to irq-injection attacks.\n");
+
 	return -1;
 }
 

  reply	other threads:[~2013-02-03 19:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-31 16:40 [PATCH] x86, x2apic: Only WARN on broken BIOSes inside a virtual guest Don Zickus
2013-01-31 18:52 ` Gleb Natapov
2013-01-31 19:34   ` Don Zickus
2013-01-31 20:00     ` Gleb Natapov
2013-01-31 20:52       ` Alex Williamson
2013-02-01 22:00         ` Andy Lutomirski
2013-02-01 22:57           ` [PATCH] intel_irq_remapping: Clean up x2apic optout security warning mess Andy Lutomirski
2013-02-03 19:29             ` tip-bot for Andy Lutomirski [this message]
2013-02-04 18:20             ` Don Zickus
2013-02-04 19:04             ` Alex Williamson
2013-02-04 19:19               ` Andy Lutomirski
2013-02-04 19:39                 ` Alex Williamson
2013-02-04 19:47                   ` Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-af8d102f999a41c0189bd2cce488bac2ee88c29b@git.kernel.org \
    --to=luto@amacapital.net \
    --cc=alex.williamson@redhat.com \
    --cc=dzickus@redhat.com \
    --cc=gleb@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=prarit@redhat.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).