[PATCH] tty: serial: pch_uart: add check for dma_alloc_coherent()

Zhaoyang Yu posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/tty/serial/pch_uart.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
[PATCH] tty: serial: pch_uart: add check for dma_alloc_coherent()
Posted by Zhaoyang Yu 2 months, 1 week ago
From: Zhaoyang Yu <2426767509@qq.com>

Add a check for dma_alloc_coherent() failure to prevent a potential
NULL pointer dereference in dma_handle_rx(). Properly release DMA
channels and the PCI device reference using a goto ladder if the
allocation fails.

Signed-off-by: Zhaoyang Yu <2426767509@qq.com>
---
 drivers/tty/serial/pch_uart.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6729d8e83c3c..ba1fcd663fe2 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -689,8 +689,7 @@ static void pch_request_dma(struct uart_port *port)
 	if (!chan) {
 		dev_err(priv->port.dev, "%s:dma_request_channel FAILS(Tx)\n",
 			__func__);
-		pci_dev_put(dma_dev);
-		return;
+		goto err_pci_get;
 	}
 	priv->chan_tx = chan;
 
@@ -704,18 +703,26 @@ static void pch_request_dma(struct uart_port *port)
 	if (!chan) {
 		dev_err(priv->port.dev, "%s:dma_request_channel FAILS(Rx)\n",
 			__func__);
-		dma_release_channel(priv->chan_tx);
-		priv->chan_tx = NULL;
-		pci_dev_put(dma_dev);
-		return;
+		goto err_req_tx;
 	}
 
 	/* Get Consistent memory for DMA */
 	priv->rx_buf_virt = dma_alloc_coherent(port->dev, port->fifosize,
 				    &priv->rx_buf_dma, GFP_KERNEL);
+	if (!priv->rx_buf_virt)
+		goto err_req_rx;
 	priv->chan_rx = chan;
 
 	pci_dev_put(dma_dev);
+	return;
+
+err_req_rx:
+	dma_release_channel(chan);
+err_req_tx:
+	dma_release_channel(priv->chan_tx);
+	priv->chan_tx = NULL;
+err_pci_get:
+	pci_dev_put(dma_dev);
 }
 
 static void pch_dma_rx_complete(void *arg)
-- 
2.50.1
Re: [PATCH] tty: serial: pch_uart: add check for dma_alloc_coherent()
Posted by Andy Shevchenko 2 months, 1 week ago
On Wed, Apr 08, 2026 at 03:24:21PM +0800, Zhaoyang Yu wrote:

> Add a check for dma_alloc_coherent() failure to prevent a potential
> NULL pointer dereference in dma_handle_rx(). Properly release DMA
> channels and the PCI device reference using a goto ladder if the
> allocation fails.

Looks like it deserves a Fixes tag.

Also ideally this driver should be converted to one of 8250 cases.
The latter has already DMA support. Note, if you are going this
direction, I have the hardware to test.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] tty: serial: pch_uart: add check for dma_alloc_coherent()
Posted by 俞朝阳 2 months ago
Hi Andy,

Thank you for the review and the suggestions!

> Looks like it deserves a Fixes tag.

Agreed. I will find the original commit that introduced this issue and add the proper Fixes tag in the v2 patch. I'll send it out shortly.

> Also ideally this driver should be converted to one of 8250 cases.
> The latter has already DMA support. Note, if you are going this
> direction, I have the hardware to test.

Thank you very much for offering to test on the actual hardware! That is incredibly helpful. 

I agree that converting this driver to the 8250 framework is the right direction. Since that would be a larger refactoring effort, I think it might be best to apply this quick NULL pointer fix first so that the current driver is safe. 

After this fix is settled, I am very interested in taking a look at the 8250 conversion as a follow-up project. I might need some time to study the 8250 DMA framework, but I will definitely reach out with a patch series for you to test when I get there!

Best regards,
Zhaoyang Yu