Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

[Rafał Miłecki]

Hi, I just updated from kernel 4.9 to 4.14 and found a NULL pointer
dereference caused by the iproc driver.

1) For bcma case pcie->dev->of_node can be NULL
2) iproc_pcie_map_dma_ranges calls pci_dma_range_parser_init
3) pci_dma_range_parser_init calls of_n_addr_cells(node) (node is NULL)

This is caused by a commit dd9d4e7498de3 ("PCI: iproc: Add inbound DMA
mapping support").

I believe that new DMA mapping code should be optional.

Would you find a moment to fix that, please?

--=20
Rafa=C5=82

Re: Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

[Ray Jui]

Hi Rafal,

Can you please give the following patch a try to see if that fixes the 
issue? Thanks.

 From f91de124bc8fb5645d7dbbfd68a9a68617955749 Mon Sep 17 00:00:00 2001
From: Ray Jui <ray.jui@broadcom.com>
Date: Tue, 2 Jan 2018 10:36:03 -0800
Subject: [PATCH] PCI: iproc: Fix NULL pointer dereference for BCMA

With the inbound DMA mapping supported added, the iProc PCIe driver
parses DT property "dma-ranges" through call to
"of_pci_dma_range_parser_init". In the case of BCMA, this results in a
NULL pointer deference due to a missing of_node.

Fix this by adding a guard in pcie-iproc-platform.c to only enable the
inbound DMA mapping logic when DT property "dma-ranges" is present

fixes: dd9d4e7498de3 ("PCI: iproc: Add inbound DMA mapping support")
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
  drivers/pci/host/pcie-iproc-platform.c | 3 +++
  drivers/pci/host/pcie-iproc.c          | 8 +++++---
  drivers/pci/host/pcie-iproc.h          | 2 ++
  3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-iproc-platform.c 
b/drivers/pci/host/pcie-iproc-platform.c
index a5073a9..235c545 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -92,6 +92,9 @@ static int iproc_pcie_pltfm_probe(struct 
platform_device *pdev)
  		pcie->need_ob_cfg = true;
  	}

+	if (of_property_read_bool(np, "dma-ranges"))
+		pcie->need_ib_cfg = true;
+
  	/* PHY use is optional */
  	pcie->phy = devm_phy_get(dev, "pcie-phy");
  	if (IS_ERR(pcie->phy)) {
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 935909b..7583606 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -1378,9 +1378,11 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, 
struct list_head *res)
  		}
  	}

-	ret = iproc_pcie_map_dma_ranges(pcie);
-	if (ret && ret != -ENOENT)
-		goto err_power_off_phy;
+	if (pcie->need_ib_cfg) {
+		ret = iproc_pcie_map_dma_ranges(pcie);
+		if (ret && ret != -ENOENT)
+			goto err_power_off_phy;
+	}

  #ifdef CONFIG_ARM
  	pcie->sysdata.private_data = pcie;
diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
index a6b55ce..4ac6282 100644
--- a/drivers/pci/host/pcie-iproc.h
+++ b/drivers/pci/host/pcie-iproc.h
@@ -74,6 +74,7 @@ struct iproc_msi;
   * @ob: outbound mapping related parameters
   * @ob_map: outbound mapping related parameters specific to the controller
   *
+ * @need_ib_cfg: indicates SW needs to configure the inbound mapping window
   * @ib: inbound mapping related parameters
   * @ib_map: outbound mapping region related parameters
   *
@@ -101,6 +102,7 @@ struct iproc_pcie {
  	struct iproc_pcie_ob ob;
  	const struct iproc_pcie_ob_map *ob_map;

+	bool need_ib_cfg;
  	struct iproc_pcie_ib ib;
  	const struct iproc_pcie_ib_map *ib_map;

-- 
2.1.4


On 12/29/2017 2:34 PM, Rafał Miłecki wrote:
> Hi, I just updated from kernel 4.9 to 4.14 and found a NULL pointer
> dereference caused by the iproc driver.
> 
> 1) For bcma case pcie->dev->of_node can be NULL
> 2) iproc_pcie_map_dma_ranges calls pci_dma_range_parser_init
> 3) pci_dma_range_parser_init calls of_n_addr_cells(node) (node is NULL)
> 
> This is caused by a commit dd9d4e7498de3 ("PCI: iproc: Add inbound DMA
> mapping support").
> 
> I believe that new DMA mapping code should be optional.
> 
> Would you find a moment to fix that, please?
> 

Re: Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

[Rafał Miłecki]

On 2 January 2018 at 19:44, Ray Jui <ray.jui@broadcom.com> wrote:
> Can you please give the following patch a try to see if that fixes the
> issue? Thanks.

It does, thank you!


> From f91de124bc8fb5645d7dbbfd68a9a68617955749 Mon Sep 17 00:00:00 2001
> From: Ray Jui <ray.jui@broadcom.com>
> Date: Tue, 2 Jan 2018 10:36:03 -0800
> Subject: [PATCH] PCI: iproc: Fix NULL pointer dereference for BCMA
>
> With the inbound DMA mapping supported added, the iProc PCIe driver
> parses DT property "dma-ranges" through call to
> "of_pci_dma_range_parser_init". In the case of BCMA, this results in a
> NULL pointer deference due to a missing of_node.
>
> Fix this by adding a guard in pcie-iproc-platform.c to only enable the
> inbound DMA mapping logic when DT property "dma-ranges" is present
>
> fixes: dd9d4e7498de3 ("PCI: iproc: Add inbound DMA mapping support")
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>

Please add:
Tested-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
Cc: stable@vger.kernel.org # 4.10+

Re: Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

[Lorenzo Pieralisi]

On Tue, Jan 02, 2018 at 11:00:43PM +0100, Rafał Miłecki wrote:
> On 2 January 2018 at 19:44, Ray Jui <ray.jui@broadcom.com> wrote:
> > Can you please give the following patch a try to see if that fixes the
> > issue? Thanks.
> 
> It does, thank you!
> 
> 
> > From f91de124bc8fb5645d7dbbfd68a9a68617955749 Mon Sep 17 00:00:00 2001
> > From: Ray Jui <ray.jui@broadcom.com>
> > Date: Tue, 2 Jan 2018 10:36:03 -0800
> > Subject: [PATCH] PCI: iproc: Fix NULL pointer dereference for BCMA
> >
> > With the inbound DMA mapping supported added, the iProc PCIe driver
> > parses DT property "dma-ranges" through call to
> > "of_pci_dma_range_parser_init". In the case of BCMA, this results in a
> > NULL pointer deference due to a missing of_node.
> >
> > Fix this by adding a guard in pcie-iproc-platform.c to only enable the
> > inbound DMA mapping logic when DT property "dma-ranges" is present
> >
> > fixes: dd9d4e7498de3 ("PCI: iproc: Add inbound DMA mapping support")
> > Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> 
> Please add:
> Tested-by: Rafał Miłecki <rafal@milecki.pl>
> Cc: stable@vger.kernel.org # 4.10+

FYI, if you want the patch to be applied please post it on linux-pci
in a separate thread.

Thanks,
Lorenzo

Re: Regression: NULL pointer dereference caused by iproc_pcie_map_dma_ranges

[Ray Jui]

Hi Lorenzo/Rafal,

A fix patch has been sent out to Bjorn and linux-pci.

Regards,

Ray

On 1/9/2018 7:54 AM, Lorenzo Pieralisi wrote:
> On Tue, Jan 02, 2018 at 11:00:43PM +0100, Rafał Miłecki wrote:
>> On 2 January 2018 at 19:44, Ray Jui <ray.jui@broadcom.com> wrote:
>>> Can you please give the following patch a try to see if that fixes the
>>> issue? Thanks.
>>
>> It does, thank you!
>>
>>
>>>  From f91de124bc8fb5645d7dbbfd68a9a68617955749 Mon Sep 17 00:00:00 2001
>>> From: Ray Jui <ray.jui@broadcom.com>
>>> Date: Tue, 2 Jan 2018 10:36:03 -0800
>>> Subject: [PATCH] PCI: iproc: Fix NULL pointer dereference for BCMA
>>>
>>> With the inbound DMA mapping supported added, the iProc PCIe driver
>>> parses DT property "dma-ranges" through call to
>>> "of_pci_dma_range_parser_init". In the case of BCMA, this results in a
>>> NULL pointer deference due to a missing of_node.
>>>
>>> Fix this by adding a guard in pcie-iproc-platform.c to only enable the
>>> inbound DMA mapping logic when DT property "dma-ranges" is present
>>>
>>> fixes: dd9d4e7498de3 ("PCI: iproc: Add inbound DMA mapping support")
>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>
>> Please add:
>> Tested-by: Rafał Miłecki <rafal@milecki.pl>
>> Cc: stable@vger.kernel.org # 4.10+
> 
> FYI, if you want the patch to be applied please post it on linux-pci
> in a separate thread.
> 
> Thanks,
> Lorenzo
>