* [PATCH v3] serial: pch_uart: potential dereference of null pointer
@ 2021-12-16 14:14 Jiasheng Jiang
2021-12-16 14:36 ` Greg KH
2021-12-20 15:37 ` Greg KH
0 siblings, 2 replies; 3+ messages in thread
From: Jiasheng Jiang @ 2021-12-16 14:14 UTC (permalink / raw)
To: gregkh, jirislaby; +Cc: linux-serial, linux-kernel, Jiasheng Jiang
The return value of dma_alloc_coherent() needs to be checked.
To avoid dereference of null pointer in case of the failure of alloc.
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
Changelog:
v2 -> v3
*Change 1. Remove dev_err.
*Change 2. Change the return type of pch_request_dma to int.
*Change 3. Return -ENOMEM when dma_alloc_coherent() failed and 0 the
others.
*Change 4. Check return value of dma_alloc_coherent().
---
drivers/tty/serial/pch_uart.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index f0351e6f0ef6..cfad5592010c 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -698,7 +698,7 @@ static bool filter(struct dma_chan *chan, void *slave)
}
}
-static void pch_request_dma(struct uart_port *port)
+static int pch_request_dma(struct uart_port *port)
{
dma_cap_mask_t mask;
struct dma_chan *chan;
@@ -723,7 +723,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__);
- return;
+ return 0;
}
priv->chan_tx = chan;
@@ -739,13 +739,20 @@ static void pch_request_dma(struct uart_port *port)
__func__);
dma_release_channel(priv->chan_tx);
priv->chan_tx = NULL;
- return;
+ return 0;
}
/* 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) {
+ dma_release_channel(priv->chan_tx);
+ priv->chan_tx = NULL;
+ return -ENOMEM;
+ }
+
priv->chan_rx = chan;
+ return 0;
}
static void pch_dma_rx_complete(void *arg)
@@ -1321,8 +1328,11 @@ static int pch_uart_startup(struct uart_port *port)
if (ret < 0)
return ret;
- if (priv->use_dma)
- pch_request_dma(port);
+ if (priv->use_dma) {
+ ret = pch_request_dma(port);
+ if (ret)
+ return ret;
+ }
priv->start_rx = 1;
pch_uart_hal_enable_interrupt(priv, PCH_UART_HAL_RX_INT |
@@ -1469,6 +1479,7 @@ static int pch_uart_verify_port(struct uart_port *port,
struct serial_struct *serinfo)
{
struct eg20t_port *priv;
+ int ret;
priv = container_of(port, struct eg20t_port, port);
if (serinfo->flags & UPF_LOW_LATENCY) {
@@ -1483,7 +1494,9 @@ static int pch_uart_verify_port(struct uart_port *port,
return -EOPNOTSUPP;
#endif
if (!priv->use_dma) {
- pch_request_dma(port);
+ ret = pch_request_dma(port);
+ if (ret)
+ return ret;
if (priv->chan_rx)
priv->use_dma = 1;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] serial: pch_uart: potential dereference of null pointer
2021-12-16 14:14 [PATCH v3] serial: pch_uart: potential dereference of null pointer Jiasheng Jiang
@ 2021-12-16 14:36 ` Greg KH
2021-12-20 15:37 ` Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-12-16 14:36 UTC (permalink / raw)
To: Jiasheng Jiang; +Cc: jirislaby, linux-serial, linux-kernel
On Thu, Dec 16, 2021 at 10:14:54PM +0800, Jiasheng Jiang wrote:
> The return value of dma_alloc_coherent() needs to be checked.
> To avoid dereference of null pointer in case of the failure of alloc.
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
A blank line is always needed before the signed-off-by line.
> ---
> Changelog:
>
> v2 -> v3
>
> *Change 1. Remove dev_err.
> *Change 2. Change the return type of pch_request_dma to int.
> *Change 3. Return -ENOMEM when dma_alloc_coherent() failed and 0 the
> others.
> *Change 4. Check return value of dma_alloc_coherent().
I see v3 here, not v4. Where is v4?
And how did you test this change?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] serial: pch_uart: potential dereference of null pointer
2021-12-16 14:14 [PATCH v3] serial: pch_uart: potential dereference of null pointer Jiasheng Jiang
2021-12-16 14:36 ` Greg KH
@ 2021-12-20 15:37 ` Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-12-20 15:37 UTC (permalink / raw)
To: Jiasheng Jiang; +Cc: jirislaby, linux-serial, linux-kernel
On Thu, Dec 16, 2021 at 10:14:54PM +0800, Jiasheng Jiang wrote:
> The return value of dma_alloc_coherent() needs to be checked.
> To avoid dereference of null pointer in case of the failure of alloc.
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
> Changelog:
>
> v2 -> v3
>
> *Change 1. Remove dev_err.
> *Change 2. Change the return type of pch_request_dma to int.
> *Change 3. Return -ENOMEM when dma_alloc_coherent() failed and 0 the
> others.
> *Change 4. Check return value of dma_alloc_coherent().
> ---
> drivers/tty/serial/pch_uart.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index f0351e6f0ef6..cfad5592010c 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -698,7 +698,7 @@ static bool filter(struct dma_chan *chan, void *slave)
> }
> }
>
> -static void pch_request_dma(struct uart_port *port)
> +static int pch_request_dma(struct uart_port *port)
> {
> dma_cap_mask_t mask;
> struct dma_chan *chan;
> @@ -723,7 +723,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__);
> - return;
> + return 0;
> }
> priv->chan_tx = chan;
>
> @@ -739,13 +739,20 @@ static void pch_request_dma(struct uart_port *port)
> __func__);
> dma_release_channel(priv->chan_tx);
> priv->chan_tx = NULL;
> - return;
> + return 0;
> }
>
> /* 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) {
> + dma_release_channel(priv->chan_tx);
> + priv->chan_tx = NULL;
> + return -ENOMEM;
> + }
> +
> priv->chan_rx = chan;
> + return 0;
> }
>
> static void pch_dma_rx_complete(void *arg)
> @@ -1321,8 +1328,11 @@ static int pch_uart_startup(struct uart_port *port)
> if (ret < 0)
> return ret;
>
> - if (priv->use_dma)
> - pch_request_dma(port);
> + if (priv->use_dma) {
> + ret = pch_request_dma(port);
> + if (ret)
> + return ret;
> + }
>
> priv->start_rx = 1;
> pch_uart_hal_enable_interrupt(priv, PCH_UART_HAL_RX_INT |
> @@ -1469,6 +1479,7 @@ static int pch_uart_verify_port(struct uart_port *port,
> struct serial_struct *serinfo)
> {
> struct eg20t_port *priv;
> + int ret;
>
> priv = container_of(port, struct eg20t_port, port);
> if (serinfo->flags & UPF_LOW_LATENCY) {
> @@ -1483,7 +1494,9 @@ static int pch_uart_verify_port(struct uart_port *port,
> return -EOPNOTSUPP;
> #endif
> if (!priv->use_dma) {
> - pch_request_dma(port);
> + ret = pch_request_dma(port);
> + if (ret)
> + return ret;
> if (priv->chan_rx)
> priv->use_dma = 1;
> }
> --
> 2.25.1
>
This patch is obviously not correct, and will cause problems if it were
accepted.
Please work on whatever tool you are using to find and make these
changes, as it is not working properly.
Or, if this was a manual change, please work on your kernel programming
skills. There are a number of bugs in this proposed change, showing
that it was not tested at all.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-20 15:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 14:14 [PATCH v3] serial: pch_uart: potential dereference of null pointer Jiasheng Jiang
2021-12-16 14:36 ` Greg KH
2021-12-20 15:37 ` Greg KH
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).