All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Marc Zyngier <maz@kernel.org>,
	x86@kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, Keith Busch <kbusch@kernel.org>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: [patch 7/7] PCI/AER: Fix the broken interrupt injection
Date: Fri, 06 Mar 2020 14:03:48 +0100	[thread overview]
Message-ID: <20200306130624.098374457@linutronix.de> (raw)
In-Reply-To: 20200306130341.199467200@linutronix.de

The AER error injection mechanism just blindly abuses generic_handle_irq()
which is really not meant for consumption by random drivers. The include of
linux/irq.h should have been a red flag in the first place. Driver code,
unless implementing interrupt chips or low level hypervisor functionality
has absolutely no business with that.

Invoking generic_handle_irq() from non interrupt handling context can have
nasty side effects at least on x86 due to the hardware trainwreck which
makes interrupt affinity changes a fragile beast. Sathyanarayanan triggered
a NULL pointer dereference in the low level APIC code that way. While the
particular pointer could be checked this would only paper over the issue
because there are other ways to trigger warnings or silently corrupt state.

Invoke the new irq_inject_interrupt() mechanism, which has the necessary
sanity checks in place and injects the interrupt via the irq_retrigger()
mechanism, which is at least halfways safe vs. the fragile x86 affinity
change mechanics.

It's safe on x86 as it does not corrupt state, but it still can cause a
premature completion of an interrupt affinity change causing the interrupt
line to become stale. Very unlikely, but possible.

For regular operations this is a non issue as AER error injection is meant
for debugging and testing and not for usage on production systems. People
using this should better know what they are doing.

Fixes: 390e2db82480 ("PCI/AER: Abstract AER interrupt handling")
Reported-by: sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/pci/pcie/Kconfig      |    1 +
 drivers/pci/pcie/aer_inject.c |    6 ++----
 2 files changed, 3 insertions(+), 4 deletions(-)

--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -34,6 +34,7 @@ config PCIEAER
 config PCIEAER_INJECT
 	tristate "PCI Express error injection support"
 	depends on PCIEAER
+	select GENERIC_IRQ_INJECTION
 	help
 	  This enables PCI Express Root Port Advanced Error Reporting
 	  (AER) software error injector.
--- a/drivers/pci/pcie/aer_inject.c
+++ b/drivers/pci/pcie/aer_inject.c
@@ -16,7 +16,7 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/irq.h>
+#include <linux/interrupt.h>
 #include <linux/miscdevice.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
@@ -468,9 +468,7 @@ static int aer_inject(struct aer_error_i
 		}
 		pci_info(edev->port, "Injecting errors %08x/%08x into device %s\n",
 			 einj->cor_status, einj->uncor_status, pci_name(dev));
-		local_irq_disable();
-		generic_handle_irq(edev->irq);
-		local_irq_enable();
+		ret = irq_inject_interrupt(edev->irq);
 	} else {
 		pci_err(rpdev, "AER device not found\n");
 		ret = -ENODEV;


  parent reply	other threads:[~2020-03-06 13:08 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 13:03 [patch 0/7] genirq/PCI: Sanitize interrupt injection Thomas Gleixner
2020-03-06 13:03 ` [patch 1/7] genirq/debugfs: Add missing sanity checks to " Thomas Gleixner
2020-03-06 13:15   ` Marc Zyngier
2020-03-07 23:20   ` Sasha Levin
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` [patch 2/7] genirq: Add protection against unsafe usage of generic_handle_irq() Thomas Gleixner
2020-03-06 13:36   ` Marc Zyngier
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` [patch 3/7] x86/apic/vector: Force interupt handler invocation to irq context Thomas Gleixner
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` [patch 4/7] genirq: Add return value to check_irq_resend() Thomas Gleixner
2020-03-06 13:44   ` Marc Zyngier
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` [patch 5/7] genirq: Sanitize state handling in check_irq_resend() Thomas Gleixner
2020-03-06 13:46   ` Marc Zyngier
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` [patch 6/7] genirq: Provide interrupt injection mechanism Thomas Gleixner
2020-03-06 13:52   ` Marc Zyngier
2020-03-06 18:34   ` Kuppuswamy Sathyanarayanan
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2020-03-06 13:03 ` Thomas Gleixner [this message]
2020-03-06 18:32   ` [patch 7/7] PCI/AER: Fix the broken interrupt injection Kuppuswamy Sathyanarayanan
2020-03-06 19:29     ` Kuppuswamy Sathyanarayanan
2020-03-08 10:14   ` [tip: irq/core] " tip-bot2 for Thomas Gleixner

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=20200306130624.098374457@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=bhelgaas@google.com \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.