* [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
@ 2019-07-19 2:54 ` Navid Emamdoost
0 siblings, 0 replies; 13+ messages in thread
From: Navid Emamdoost @ 2019-07-19 2:54 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Greg Kroah-Hartman,
Jiri Slaby, Andy Shevchenko, Vinod Koul, linux-serial,
linux-kernel
pci_ioremap_bar may return null. This is eventually de-referenced at
drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check is
needed to prevent null de-reference. I am adding the check and in case of
failure returning -ENOMEM (I am not sure this is the best errno, you may
consider it as a placeholder), and subsequently changing the caller’s
return type, and propagating the error.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/tty/serial/8250/8250_lpss.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 53ca9ba6ab4b..5954b2e09b76 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -161,7 +161,7 @@ static const struct dw_dma_platform_data qrk_serial_dma_pdata = {
.multi_block = {0},
};
-static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
+static int qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
{
struct uart_8250_dma *dma = &lpss->dma;
struct dw_dma_chip *chip = &lpss->dma_chip;
@@ -172,12 +172,14 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
chip->dev = &pdev->dev;
chip->irq = pci_irq_vector(pdev, 0);
chip->regs = pci_ioremap_bar(pdev, 1);
+ if (!chip->regs)
+ return -ENOMEM;
chip->pdata = &qrk_serial_dma_pdata;
/* Falling back to PIO mode if DMA probing fails */
ret = dw_dma_probe(chip);
if (ret)
- return;
+ return 0;
pci_try_set_mwi(pdev);
@@ -191,6 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
param->hs_polarity = true;
lpss->dma_maxburst = 8;
+ return 0;
}
static void qrk_serial_exit_dma(struct lpss8250 *lpss)
@@ -219,7 +222,9 @@ static int qrk_serial_setup(struct lpss8250 *lpss, struct uart_port *port)
port->irq = pci_irq_vector(pdev, 0);
- qrk_serial_setup_dma(lpss, port);
+ ret = qrk_serial_setup_dma(lpss, port);
+ if (ret < 0)
+ return ret;
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
@ 2019-07-19 2:54 ` Navid Emamdoost
0 siblings, 0 replies; 13+ messages in thread
From: Navid Emamdoost @ 2019-07-19 2:54 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Greg Kroah-Hartman,
Jiri Slaby, Andy Shevchenko, Vinod Koul, linux-serial,
linux-kernel
pci_ioremap_bar may return null. This is eventually de-referenced at
drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check is
needed to prevent null de-reference. I am adding the check and in case of
failure returning -ENOMEM (I am not sure this is the best errno, you may
consider it as a placeholder), and subsequently changing the caller’s
return type, and propagating the error.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/tty/serial/8250/8250_lpss.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 53ca9ba6ab4b..5954b2e09b76 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -161,7 +161,7 @@ static const struct dw_dma_platform_data qrk_serial_dma_pdata = {
.multi_block = {0},
};
-static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
+static int qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
{
struct uart_8250_dma *dma = &lpss->dma;
struct dw_dma_chip *chip = &lpss->dma_chip;
@@ -172,12 +172,14 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
chip->dev = &pdev->dev;
chip->irq = pci_irq_vector(pdev, 0);
chip->regs = pci_ioremap_bar(pdev, 1);
+ if (!chip->regs)
+ return -ENOMEM;
chip->pdata = &qrk_serial_dma_pdata;
/* Falling back to PIO mode if DMA probing fails */
ret = dw_dma_probe(chip);
if (ret)
- return;
+ return 0;
pci_try_set_mwi(pdev);
@@ -191,6 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
param->hs_polarity = true;
lpss->dma_maxburst = 8;
+ return 0;
}
static void qrk_serial_exit_dma(struct lpss8250 *lpss)
@@ -219,7 +222,9 @@ static int qrk_serial_setup(struct lpss8250 *lpss, struct uart_port *port)
port->irq = pci_irq_vector(pdev, 0);
- qrk_serial_setup_dma(lpss, port);
+ ret = qrk_serial_setup_dma(lpss, port);
+ if (ret < 0)
+ return ret;
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-19 2:54 ` Navid Emamdoost
(?)
@ 2019-07-19 13:37 ` Andy Shevchenko
2019-07-19 15:15 ` Andy Shevchenko
-1 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2019-07-19 13:37 UTC (permalink / raw)
To: Navid Emamdoost, Kangjie Lu, Aditya Pakki
Cc: emamd001, smccaman, Greg Kroah-Hartman, Jiri Slaby, Vinod Koul,
linux-serial, linux-kernel
On Thu, Jul 18, 2019 at 09:54:42PM -0500, Navid Emamdoost wrote:
> pci_ioremap_bar may return null. This is eventually de-referenced at
> drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check is
> needed to prevent null de-reference. I am adding the check and in case of
> failure returning -ENOMEM (I am not sure this is the best errno, you may
> consider it as a placeholder), and subsequently changing the caller’s
> return type, and propagating the error.
Thanks for the patch, my comments below.
> chip->irq = pci_irq_vector(pdev, 0);
> chip->regs = pci_ioremap_bar(pdev, 1);
> + if (!chip->regs)
> + return -ENOMEM;
This is the same case as below, it's fine to go on without DMA support.
> chip->pdata = &qrk_serial_dma_pdata;
So, I would rather to put like this...
Hold on, I remember someone already tried to fix this [1].
I dunno why it wasn't v5, due to [2].
Also, similar to yours, but wrong [3].
Thus, please, collaborate guys, and send one compiling solution based on [1].
> /* Falling back to PIO mode if DMA probing fails */
> ret = dw_dma_probe(chip);
> if (ret)
> - return;
> + return 0;
[1]: https://www.spinics.net/lists/linux-serial/msg33965.html
[2]: https://lists.01.org/pipermail/kbuild-all/2019-March/059215.html
[3]: https://lore.kernel.org/patchwork/patch/1051000/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-19 2:54 ` Navid Emamdoost
@ 2019-07-19 13:58 ` kbuild test robot
-1 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2019-07-19 13:58 UTC (permalink / raw)
To: Navid Emamdoost
Cc: kbuild-all, emamd001, kjlu, smccaman, Navid Emamdoost,
Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Vinod Koul,
linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
Hi Navid,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.2 next-20190718]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Navid-Emamdoost/8250_lpss-check-null-return-when-calling-pci_ioremap_bar/20190719-120441
config: x86_64-randconfig-h002-201928 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/tty/serial/8250/8250_lpss.c: In function 'qrk_serial_setup':
>> drivers/tty/serial/8250/8250_lpss.c:225:6: error: void value not ignored as it ought to be
ret = qrk_serial_setup_dma(lpss, port);
^
vim +225 drivers/tty/serial/8250/8250_lpss.c
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31667 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
@ 2019-07-19 13:58 ` kbuild test robot
0 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2019-07-19 13:58 UTC (permalink / raw)
Cc: kbuild-all, emamd001, kjlu, smccaman, Navid Emamdoost,
Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, Vinod Koul,
linux-serial, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
Hi Navid,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.2 next-20190718]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Navid-Emamdoost/8250_lpss-check-null-return-when-calling-pci_ioremap_bar/20190719-120441
config: x86_64-randconfig-h002-201928 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/tty/serial/8250/8250_lpss.c: In function 'qrk_serial_setup':
>> drivers/tty/serial/8250/8250_lpss.c:225:6: error: void value not ignored as it ought to be
ret = qrk_serial_setup_dma(lpss, port);
^
vim +225 drivers/tty/serial/8250/8250_lpss.c
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31667 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-19 13:37 ` Andy Shevchenko
@ 2019-07-19 15:15 ` Andy Shevchenko
2019-07-19 17:48 ` [PATCH v2] " Navid Emamdoost
0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2019-07-19 15:15 UTC (permalink / raw)
To: Navid Emamdoost, Kangjie Lu, Aditya Pakki
Cc: emamd001, smccaman, Greg Kroah-Hartman, Jiri Slaby, Vinod Koul,
linux-serial, linux-kernel
On Fri, Jul 19, 2019 at 04:37:35PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 18, 2019 at 09:54:42PM -0500, Navid Emamdoost wrote:
> Thus, please, collaborate guys, and send one compiling solution based on [1].
> [1]: https://www.spinics.net/lists/linux-serial/msg33965.html
For your convenience (what has to be applied to v4):
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 53ca9ba6ab4b..6214da67e9b3 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -195,11 +195,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
static void qrk_serial_exit_dma(struct lpss8250 *lpss)
{
+ struct dw_dma_chip *chip = &lpss->dma_chip;
struct dw_dma_slave *param = &lpss->dma_param;
if (!param->dma_dev)
return;
- dw_dma_remove(&lpss->dma_chip);
+
+ dw_dma_remove(chip);
+
+ pci_iounmap(to_pci_dev(chip->dev), chip->regs);
}
#else /* CONFIG_SERIAL_8250_DMA */
static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port) {}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-19 15:15 ` Andy Shevchenko
@ 2019-07-19 17:48 ` Navid Emamdoost
2019-07-19 22:36 ` Andy Shevchenko
[not found] ` <CGME20190726113223eucas1p287f8f2df03f66658bd492c592fd426e6@eucas1p2.samsung.com>
0 siblings, 2 replies; 13+ messages in thread
From: Navid Emamdoost @ 2019-07-19 17:48 UTC (permalink / raw)
To: andriy.shevchenko
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, linux-serial, linux-kernel
pci_ioremap_bar may return null. This is eventually de-referenced at
drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check
is needed to prevent null de-reference. I am adding the check and in case
of failure. Thanks to Andy Shevchenko for the hint on the necessity of
pci_iounmap when exiting.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/tty/serial/8250/8250_lpss.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 53ca9ba6ab4b..d07e431110d9 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -169,10 +169,12 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
struct pci_dev *pdev = to_pci_dev(port->dev);
int ret;
+ chip->pdata = &qrk_serial_dma_pdata;
chip->dev = &pdev->dev;
chip->irq = pci_irq_vector(pdev, 0);
chip->regs = pci_ioremap_bar(pdev, 1);
- chip->pdata = &qrk_serial_dma_pdata;
+ if (!chip->regs)
+ return;
/* Falling back to PIO mode if DMA probing fails */
ret = dw_dma_probe(chip);
@@ -195,11 +197,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
static void qrk_serial_exit_dma(struct lpss8250 *lpss)
{
+ struct dw_dma_chip *chip = &lpss->dma_chip;
struct dw_dma_slave *param = &lpss->dma_param;
if (!param->dma_dev)
return;
- dw_dma_remove(&lpss->dma_chip);
+
+ dw_dma_remove(chip);
+
+ pci_iounmap(to_pci_dev(chip->dev), chip->regs);
}
#else /* CONFIG_SERIAL_8250_DMA */
static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port) {}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-19 17:48 ` [PATCH v2] " Navid Emamdoost
@ 2019-07-19 22:36 ` Andy Shevchenko
[not found] ` <CGME20190726113223eucas1p287f8f2df03f66658bd492c592fd426e6@eucas1p2.samsung.com>
1 sibling, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2019-07-19 22:36 UTC (permalink / raw)
To: Navid Emamdoost
Cc: Andy Shevchenko, emamd001, Kangjie Lu, smccaman,
Greg Kroah-Hartman, Jiri Slaby, Vinod Koul,
open list:SERIAL DRIVERS, Linux Kernel Mailing List
On Sat, Jul 20, 2019 at 12:45 AM Navid Emamdoost
<navid.emamdoost@gmail.com> wrote:
>
> pci_ioremap_bar may return null. This is eventually de-referenced at
> drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check
> is needed to prevent null de-reference. I am adding the check and in case
> of failure. Thanks to Andy Shevchenko for the hint on the necessity of
> pci_iounmap when exiting.
>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> drivers/tty/serial/8250/8250_lpss.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index 53ca9ba6ab4b..d07e431110d9 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -169,10 +169,12 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
> struct pci_dev *pdev = to_pci_dev(port->dev);
> int ret;
>
> + chip->pdata = &qrk_serial_dma_pdata;
> chip->dev = &pdev->dev;
> chip->irq = pci_irq_vector(pdev, 0);
> chip->regs = pci_ioremap_bar(pdev, 1);
> - chip->pdata = &qrk_serial_dma_pdata;
> + if (!chip->regs)
> + return;
>
> /* Falling back to PIO mode if DMA probing fails */
> ret = dw_dma_probe(chip);
> @@ -195,11 +197,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
>
> static void qrk_serial_exit_dma(struct lpss8250 *lpss)
> {
> + struct dw_dma_chip *chip = &lpss->dma_chip;
> struct dw_dma_slave *param = &lpss->dma_param;
>
> if (!param->dma_dev)
> return;
> - dw_dma_remove(&lpss->dma_chip);
> +
> + dw_dma_remove(chip);
> +
> + pci_iounmap(to_pci_dev(chip->dev), chip->regs);
> }
> #else /* CONFIG_SERIAL_8250_DMA */
> static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port) {}
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] 8250_lpss: check null return when calling pci_ioremap_bar
[not found] ` <CGME20190726113223eucas1p287f8f2df03f66658bd492c592fd426e6@eucas1p2.samsung.com>
@ 2019-07-26 11:32 ` Bartlomiej Zolnierkiewicz
2019-07-26 11:57 ` Andy Shevchenko
0 siblings, 1 reply; 13+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-07-26 11:32 UTC (permalink / raw)
To: Navid Emamdoost
Cc: andriy.shevchenko, emamd001, kjlu, smccaman, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, linux-serial, linux-kernel
Hi,
On 7/19/19 7:48 PM, Navid Emamdoost wrote:
> pci_ioremap_bar may return null. This is eventually de-referenced at
> drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check
> is needed to prevent null de-reference. I am adding the check and in case
> of failure. Thanks to Andy Shevchenko for the hint on the necessity of
> pci_iounmap when exiting.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> drivers/tty/serial/8250/8250_lpss.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index 53ca9ba6ab4b..d07e431110d9 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -169,10 +169,12 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
> struct pci_dev *pdev = to_pci_dev(port->dev);
> int ret;
>
> + chip->pdata = &qrk_serial_dma_pdata;
> chip->dev = &pdev->dev;
> chip->irq = pci_irq_vector(pdev, 0);
> chip->regs = pci_ioremap_bar(pdev, 1);
> - chip->pdata = &qrk_serial_dma_pdata;
> + if (!chip->regs)
> + return;
>
> /* Falling back to PIO mode if DMA probing fails */
> ret = dw_dma_probe(chip);
pci_iounmap() should also be called on dw_dma_probe() failure (in such
case param->dma_dev is NULL so pci_iounmap() in qrk_serial_exit_dma()
won't be called during exit).
> @@ -195,11 +197,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
> pci_iounmap
> static void qrk_serial_exit_dma(struct lpss8250 *lpss)
> {
> + struct dw_dma_chip *chip = &lpss->dma_chip;
> struct dw_dma_slave *param = &lpss->dma_param;
>
> if (!param->dma_dev)
> return;
> - dw_dma_remove(&lpss->dma_chip);
> +
> + dw_dma_remove(chip);
> +
> + pci_iounmap(to_pci_dev(chip->dev), chip->regs);
> }
> #else /* CONFIG_SERIAL_8250_DMA */
> static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port) {}
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-26 11:32 ` Bartlomiej Zolnierkiewicz
@ 2019-07-26 11:57 ` Andy Shevchenko
2019-07-26 19:53 ` [PATCH v3] " Navid Emamdoost
0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2019-07-26 11:57 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Navid Emamdoost, emamd001, kjlu, smccaman, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, linux-serial, linux-kernel
On Fri, Jul 26, 2019 at 01:32:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On 7/19/19 7:48 PM, Navid Emamdoost wrote:
> > pci_ioremap_bar may return null. This is eventually de-referenced at
> > drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check
> > is needed to prevent null de-reference. I am adding the check and in case
> > of failure. Thanks to Andy Shevchenko for the hint on the necessity of
> > pci_iounmap when exiting.
> > @@ -169,10 +169,12 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
> > struct pci_dev *pdev = to_pci_dev(port->dev);
> > int ret;
> >
> > + chip->pdata = &qrk_serial_dma_pdata;
> > chip->dev = &pdev->dev;
> > chip->irq = pci_irq_vector(pdev, 0);
> > chip->regs = pci_ioremap_bar(pdev, 1);
> > - chip->pdata = &qrk_serial_dma_pdata;
> > + if (!chip->regs)
> > + return;
> >
> > /* Falling back to PIO mode if DMA probing fails */
> > ret = dw_dma_probe(chip);
>
> pci_iounmap() should also be called on dw_dma_probe() failure (in such
> case param->dma_dev is NULL so pci_iounmap() in qrk_serial_exit_dma()
> won't be called during exit).
Oh, yes, good catch!
Navid, can you send a follow up fix?
> > @@ -195,11 +197,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
> > pci_iounmap
> > static void qrk_serial_exit_dma(struct lpss8250 *lpss)
> > {
> > + struct dw_dma_chip *chip = &lpss->dma_chip;
> > struct dw_dma_slave *param = &lpss->dma_param;
> >
> > if (!param->dma_dev)
> > return;
> > - dw_dma_remove(&lpss->dma_chip);
> > +
> > + dw_dma_remove(chip);
> > +
> > + pci_iounmap(to_pci_dev(chip->dev), chip->regs);
> > }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] 8250_lpss: check null return when calling pci_ioremap_bar
2019-07-26 11:57 ` Andy Shevchenko
@ 2019-07-26 19:53 ` Navid Emamdoost
[not found] ` <CAEkB2ERhxLj7ogoy1E3j8d4MyEZqroWS1tPRxyJXR2oLhNz+LQ@mail.gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Navid Emamdoost @ 2019-07-26 19:53 UTC (permalink / raw)
To: andriy.shevchenko
Cc: emamd001, kjlu, b.zolnierkie, gregkh, smccaman, secalert,
Navid Emamdoost, Jiri Slaby, Vinod Koul, linux-serial,
linux-kernel
pci_ioremap_bar may return null. This is eventually de-referenced at
drivers/dma/dw/core.c:1154 and drivers/dma/dw/core.c:1168. A null check
is needed to prevent null de-reference. I am adding the check and in case
of failure. Thanks to Andy Shevchenko for the hint on the necessity of
pci_iounmap when exiting.
Update: also covered the dw_dma_probe() failure.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/tty/serial/8250/8250_lpss.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 53ca9ba6ab4b..19b356119ef6 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -169,15 +169,20 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
struct pci_dev *pdev = to_pci_dev(port->dev);
int ret;
+ chip->pdata = &qrk_serial_dma_pdata;
chip->dev = &pdev->dev;
chip->irq = pci_irq_vector(pdev, 0);
chip->regs = pci_ioremap_bar(pdev, 1);
- chip->pdata = &qrk_serial_dma_pdata;
+ if (!chip->regs)
+ return;
/* Falling back to PIO mode if DMA probing fails */
ret = dw_dma_probe(chip);
- if (ret)
+ if (ret) {
+ dw_dma_remove(chip);
+ pci_iounmap(to_pci_dev(chip->dev), chip->regs);
return;
+ }
pci_try_set_mwi(pdev);
@@ -195,11 +200,15 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
static void qrk_serial_exit_dma(struct lpss8250 *lpss)
{
+ struct dw_dma_chip *chip = &lpss->dma_chip;
struct dw_dma_slave *param = &lpss->dma_param;
if (!param->dma_dev)
return;
- dw_dma_remove(&lpss->dma_chip);
+
+ dw_dma_remove(chip);
+
+ pci_iounmap(to_pci_dev(chip->dev), chip->regs);
}
#else /* CONFIG_SERIAL_8250_DMA */
static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port) {}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] 8250_lpss: check null return when calling pci_ioremap_bar
[not found] ` <CAEkB2ERhxLj7ogoy1E3j8d4MyEZqroWS1tPRxyJXR2oLhNz+LQ@mail.gmail.com>
@ 2019-09-07 15:53 ` Andy Shevchenko
[not found] ` <CAEkB2ET-SQFiBbroxDFEVrPxto6a2wLJf0NM7R=ERcPargr66Q@mail.gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2019-09-07 15:53 UTC (permalink / raw)
To: Navid Emamdoost
Cc: Navid Emamdoost, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, linux-serial, linux-kernel
On Fri, Sep 06, 2019 at 05:32:39PM -0500, Navid Emamdoost wrote:
> I was wondering is anyone reviewing this patch?
> https://lore.kernel.org/patchwork/patch/1106267/
Why?
The one below is a part of upstraem
commit f5d6aadf3b6434f11393e33be9fd25a56d0bc872
Author: Navid Emamdoost <navid.emamdoost@gmail.com>
Date: Fri Jul 19 12:48:45 2019 -0500
8250_lpss: check null return when calling pci_ioremap_bar
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] 8250_lpss: check null return when calling pci_ioremap_bar
[not found] ` <CAEkB2ET-SQFiBbroxDFEVrPxto6a2wLJf0NM7R=ERcPargr66Q@mail.gmail.com>
@ 2019-09-07 16:16 ` Andy Shevchenko
0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2019-09-07 16:16 UTC (permalink / raw)
To: Navid Emamdoost
Cc: Navid Emamdoost, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, linux-serial, linux-kernel
On Sat, Sep 07, 2019 at 11:06:29AM -0500, Navid Emamdoost wrote:
> Cool! I thought it is forgotten, as there were no response to the v3 of the
> patch.
Nevertheless, you may send a follow up, since one call to pci_iounmap() is
missed when dw_dma_probe() fails.
> Thanks for letting me know.
You're welcome.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-09-07 16:16 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 2:54 [PATCH] 8250_lpss: check null return when calling pci_ioremap_bar Navid Emamdoost
2019-07-19 2:54 ` Navid Emamdoost
2019-07-19 13:37 ` Andy Shevchenko
2019-07-19 15:15 ` Andy Shevchenko
2019-07-19 17:48 ` [PATCH v2] " Navid Emamdoost
2019-07-19 22:36 ` Andy Shevchenko
[not found] ` <CGME20190726113223eucas1p287f8f2df03f66658bd492c592fd426e6@eucas1p2.samsung.com>
2019-07-26 11:32 ` Bartlomiej Zolnierkiewicz
2019-07-26 11:57 ` Andy Shevchenko
2019-07-26 19:53 ` [PATCH v3] " Navid Emamdoost
[not found] ` <CAEkB2ERhxLj7ogoy1E3j8d4MyEZqroWS1tPRxyJXR2oLhNz+LQ@mail.gmail.com>
2019-09-07 15:53 ` Andy Shevchenko
[not found] ` <CAEkB2ET-SQFiBbroxDFEVrPxto6a2wLJf0NM7R=ERcPargr66Q@mail.gmail.com>
2019-09-07 16:16 ` Andy Shevchenko
2019-07-19 13:58 ` [PATCH] " kbuild test robot
2019-07-19 13:58 ` kbuild test robot
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.