Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610945 > unrolled thread

[PATCH 4.4 74/76] serial: 8250_pci: Detach low-level driver during PCI error recovery

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4.4 74/76] serial: 8250_pci: Detach low-level driver during PCI error recovery Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-28 15:50 +0200
    Re: [PATCH 4.4 74/76] serial: 8250_pci: Detach low-level driver  during PCI error recovery Ben Hutchings <ben.hutchings@codethink.co.uk> - 2017-04-04 22:30 +0200

#1610945 — [PATCH 4.4 74/76] serial: 8250_pci: Detach low-level driver during PCI error recovery

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Sumit Semwal <sumit.semwal@linaro.org>


From: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>

[ Upstream commit f209fa03fc9d131b3108c2e4936181eabab87416 ]

During a PCI error recovery, like the ones provoked by EEH in the ppc64
platform, all IO to the device must be blocked while the recovery is
completed.  Current 8250_pci implementation only suspends the port
instead of detaching it, which doesn't prevent incoming accesses like
TIOCMGET and TIOCMSET calls from reaching the device.  Those end up
racing with the EEH recovery, crashing it.  Similar races were also
observed when opening the device and when shutting it down during
recovery.

This patch implements a more robust IO blockage for the 8250_pci
recovery by unregistering the port at the beginning of the procedure and
re-adding it afterwards.  Since the port is detached from the uart
layer, we can be sure that no request will make through to the device
during recovery.  This is similar to the solution used by the JSM serial
driver.

I thank Peter Hurley <peter@hurleysoftware.com> for valuable input on
this one over one year ago.

Signed-off-by: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_pci.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -57,6 +57,7 @@ struct serial_private {
 	unsigned int		nr;
 	void __iomem		*remapped_bar[PCI_NUM_BAR_RESOURCES];
 	struct pci_serial_quirk	*quirk;
+	const struct pciserial_board *board;
 	int			line[0];
 };
 
@@ -4058,6 +4059,7 @@ pciserial_init_ports(struct pci_dev *dev
 		}
 	}
 	priv->nr = i;
+	priv->board = board;
 	return priv;
 
 err_deinit:
@@ -4068,7 +4070,7 @@ err_out:
 }
 EXPORT_SYMBOL_GPL(pciserial_init_ports);
 
-void pciserial_remove_ports(struct serial_private *priv)
+void pciserial_detach_ports(struct serial_private *priv)
 {
 	struct pci_serial_quirk *quirk;
 	int i;
@@ -4088,7 +4090,11 @@ void pciserial_remove_ports(struct seria
 	quirk = find_quirk(priv->dev);
 	if (quirk->exit)
 		quirk->exit(priv->dev);
+}
 
+void pciserial_remove_ports(struct serial_private *priv)
+{
+	pciserial_detach_ports(priv);
 	kfree(priv);
 }
 EXPORT_SYMBOL_GPL(pciserial_remove_ports);
@@ -5819,7 +5825,7 @@ static pci_ers_result_t serial8250_io_er
 		return PCI_ERS_RESULT_DISCONNECT;
 
 	if (priv)
-		pciserial_suspend_ports(priv);
+		pciserial_detach_ports(priv);
 
 	pci_disable_device(dev);
 
@@ -5844,9 +5850,18 @@ static pci_ers_result_t serial8250_io_sl
 static void serial8250_io_resume(struct pci_dev *dev)
 {
 	struct serial_private *priv = pci_get_drvdata(dev);
+	const struct pciserial_board *board;
 
-	if (priv)
-		pciserial_resume_ports(priv);
+	if (!priv)
+		return;
+
+	board = priv->board;
+	kfree(priv);
+	priv = pciserial_init_ports(dev, board);
+
+	if (!IS_ERR(priv)) {
+		pci_set_drvdata(dev, priv);
+	}
 }
 
 static const struct pci_error_handlers serial8250_err_handler = {

[toc] | [next] | [standalone]

#1616373 — Re: [PATCH 4.4 74/76] serial: 8250_pci: Detach low-level driver during PCI error recovery

On Tue, 2017-03-28 at 14:31 +0200, Greg Kroah-Hartman wrote:
[...]
>  static void serial8250_io_resume(struct pci_dev *dev)
>  {
>  	struct serial_private *priv = pci_get_drvdata(dev);
> +	const struct pciserial_board *board;
>  
> -	if (priv)
> -		pciserial_resume_ports(priv);
> +	if (!priv)
> +		return;
> +
> +	board = priv->board;
> +	kfree(priv);
> +	priv = pciserial_init_ports(dev, board);
> +
> +	if (!IS_ERR(priv)) {
> +		pci_set_drvdata(dev, priv);
> +	}
>  }

On error, this leaves drvdata as a dangling pointer.  Removing the
device or driver will then cause a use-after-free.  (And setting drvdata
to NULL isn't enough to fix this as there is no null pointer check in
pciserial_remove_ports().)

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web