[PATCH] PCI: dwc: keystone: Fix potential NULL dereference

[PATCH] PCI: dwc: keystone: Fix potential NULL dereference

[Aleksandr Mishin]

In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
NULL which is later dereferenced. Fix this bug by adding NULL check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
 drivers/pci/controller/dwc/pci-keystone.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 844de4418724..00d616654171 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -392,7 +392,11 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	struct resource *mem;
 	int i;
 
-	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
+	struct resource_entry *ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+	if (!ft)
+		return;
+
+	mem = ft->res;
 	start = mem->start;
 	end = mem->end;
 
-- 
2.30.2

Re: [PATCH] PCI: dwc: keystone: Fix potential NULL dereference

[Bjorn Helgaas]

On Fri, Mar 29, 2024 at 08:19:47AM +0300, Aleksandr Mishin wrote:
> In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> NULL which is later dereferenced. Fix this bug by adding NULL check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..00d616654171 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -392,7 +392,11 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	struct resource *mem;
>  	int i;
>  
> -	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
> +	struct resource_entry *ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if (!ft)
> +		return;

Like the other one
(https://lore.kernel.org/all/20240328180126.23574-1-amishin@t-argos.ru/),
I think this potentially avoids a NULL pointer dereference (though I
didn't do the analysis in this case to see whether that's actually
possible), but fails to consider the implication of simply skipping
the rest of ks_pcie_setup_rc_app_regs().

"start" and "end" are used only for the loop about "Using Direct 1:1
mapping of RC <-> PCI memory space".  If there's no IORESOURCE_MEM
resource, obviously that loop makes no sense.  The function would
probably be improved by moving the resource_list_first_type() so it's
next to the loop that uses the result.

But the rest of the function doesn't depend on that IORESOURCE_MEM
resource, and it's not at all clear that the rest of the function
should be skipped.

> +	mem = ft->res;
>  	start = mem->start;
>  	end = mem->end;
>  
> -- 
> 2.30.2
> 

[PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Aleksandr Mishin]

In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
NULL which is later dereferenced. Fix this bug by adding NULL check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
v2: Add return code processing

 drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 844de4418724..5c6786d9f3e9 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
 	} while (val & DBI_CS2);
 }
 
-static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
+static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	u32 val;
 	u32 num_viewport = ks_pcie->num_viewport;
 	struct dw_pcie *pci = ks_pcie->pci;
 	struct dw_pcie_rp *pp = &pci->pp;
-	u64 start, end;
+	struct resource_entry *ft;
 	struct resource *mem;
+	u64 start, end;
 	int i;
 
-	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
+	ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+	if (!ft)
+		return -EINVAL;
+
+	mem = ft->res;
 	start = mem->start;
 	end = mem->end;
 
@@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	ks_pcie_clear_dbi_mode(ks_pcie);
 
 	if (ks_pcie->is_am6)
-		return;
+		return 0;
 
 	val = ilog2(OB_WIN_SIZE);
 	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
@@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
 	val |= OB_XLAT_EN_VAL;
 	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
+
+	return 0;
 }
 
 static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
@@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 		return ret;
 
 	ks_pcie_stop_link(pci);
-	ks_pcie_setup_rc_app_regs(ks_pcie);
+	ret = ks_pcie_setup_rc_app_regs(ks_pcie);
+	if (ret < 0)
+		return ret;
+
 	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
 			pci->dbi_base + PCI_IO_BASE);
 
-- 
2.30.2

Re: [PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Alexander Lobakin]

From: Aleksandr Mishin <amishin@t-argos.ru>
Date: Thu, 25 Apr 2024 12:21:35 +0300

> In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> NULL which is later dereferenced. Fix this bug by adding NULL check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.

Please stop spamming with "potential fixes" made mechanically from
static analyzer reports without looking into the code flow. These
patches are mostly incorrect and may hurt.
Either have a stable repro and then fix the real bug or don't touch
anything at all.

> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>

Thanks,
Olek

Re: [PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Bjorn Helgaas]

On Thu, Apr 25, 2024 at 03:00:14PM +0200, Alexander Lobakin wrote:
> From: Aleksandr Mishin <amishin@t-argos.ru>
> Date: Thu, 25 Apr 2024 12:21:35 +0300
> 
> > In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> > NULL which is later dereferenced. Fix this bug by adding NULL check.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Please stop spamming with "potential fixes" made mechanically from
> static analyzer reports without looking into the code flow. These
> patches are mostly incorrect and may hurt.
> Either have a stable repro and then fix the real bug or don't touch
> anything at all.

Did you look at the actual patch?  I'm not a keystone expert, but this
patch looks reasonable to me.

It might still be the case that we're guaranteed to have an
IORESOURCE_MEM window by other code, but it looks like a real hassle
to prove that.

> > Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> > Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> 
> Thanks,
> Olek

Re: [PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Manivannan Sadhasivam]

On Thu, Apr 25, 2024 at 03:00:14PM +0200, Alexander Lobakin wrote:
> From: Aleksandr Mishin <amishin@t-argos.ru>
> Date: Thu, 25 Apr 2024 12:21:35 +0300
> 
> > In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> > NULL which is later dereferenced. Fix this bug by adding NULL check.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Please stop spamming with "potential fixes" made mechanically from
> static analyzer reports without looking into the code flow. These
> patches are mostly incorrect and may hurt.
> Either have a stable repro and then fix the real bug or don't touch
> anything at all.
> 

This patch obviously fixes the potential issue where resource_list_first_type()
may return NULL if the MEM range is not provided in DT.
pci_parse_request_of_pci_ranges() will just emit a warning in that case and this
code path will cause a NULL pointer dereference.

Even though this situation means that the DT is broken, it still makes sense to
have the checks in place.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Manivannan Sadhasivam]

On Thu, Apr 25, 2024 at 12:21:35PM +0300, Aleksandr Mishin wrote:
> In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> NULL which is later dereferenced. Fix this bug by adding NULL check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> ---
> v2: Add return code processing
> 
>  drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..5c6786d9f3e9 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
>  	} while (val & DBI_CS2);
>  }
>  
> -static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
> +static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  {
>  	u32 val;
>  	u32 num_viewport = ks_pcie->num_viewport;
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct dw_pcie_rp *pp = &pci->pp;
> -	u64 start, end;
> +	struct resource_entry *ft;
>  	struct resource *mem;
> +	u64 start, end;
>  	int i;
>  
> -	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
> +	ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if (!ft)
> +		return -EINVAL;

-ENODEV please.

- Mani

> +
> +	mem = ft->res;
>  	start = mem->start;
>  	end = mem->end;
>  
> @@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	ks_pcie_clear_dbi_mode(ks_pcie);
>  
>  	if (ks_pcie->is_am6)
> -		return;
> +		return 0;
>  
>  	val = ilog2(OB_WIN_SIZE);
>  	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
> @@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
>  	val |= OB_XLAT_EN_VAL;
>  	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
> +
> +	return 0;
>  }
>  
>  static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
> @@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
>  		return ret;
>  
>  	ks_pcie_stop_link(pci);
> -	ks_pcie_setup_rc_app_regs(ks_pcie);
> +	ret = ks_pcie_setup_rc_app_regs(ks_pcie);
> +	if (ret < 0)
> +		return ret;
> +
>  	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
>  			pci->dbi_base + PCI_IO_BASE);
>  
> -- 
> 2.30.2
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] PCI: dwc: keystone: Fix potential NULL dereference

[Niklas Cassel]

On Thu, Apr 25, 2024 at 12:21:35PM +0300, Aleksandr Mishin wrote:
> In ks_pcie_setup_rc_app_regs() resource_list_first_type() may return
> NULL which is later dereferenced. Fix this bug by adding NULL check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
> ---
> v2: Add return code processing
> 
>  drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..5c6786d9f3e9 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
>  	} while (val & DBI_CS2);
>  }
>  
> -static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
> +static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  {
>  	u32 val;
>  	u32 num_viewport = ks_pcie->num_viewport;
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct dw_pcie_rp *pp = &pci->pp;
> -	u64 start, end;
> +	struct resource_entry *ft;
>  	struct resource *mem;
> +	u64 start, end;
>  	int i;
>  
> -	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
> +	ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if (!ft)
> +		return -EINVAL;
> +
> +	mem = ft->res;
>  	start = mem->start;
>  	end = mem->end;
>  
> @@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	ks_pcie_clear_dbi_mode(ks_pcie);
>  
>  	if (ks_pcie->is_am6)
> -		return;
> +		return 0;
>  
>  	val = ilog2(OB_WIN_SIZE);
>  	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
> @@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
>  	val |= OB_XLAT_EN_VAL;
>  	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
> +
> +	return 0;
>  }
>  
>  static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
> @@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
>  		return ret;
>  
>  	ks_pcie_stop_link(pci);
> -	ks_pcie_setup_rc_app_regs(ks_pcie);
> +	ret = ks_pcie_setup_rc_app_regs(ks_pcie);

Since ks_pcie_setup_rc_app_regs() returns 0 on success,
I suggest you do:

if (ret)
	return ret;

Instead.

> +	if (ret < 0)
> +		return ret;
> +


Kind regards,
Niklas

[PATCH v3] PCI: dwc: keystone: Fix NULL pointer dereference in case of DT error in ks_pcie_setup_rc_app_regs()

[Aleksandr Mishin]

If IORESOURCE_MEM is not provided in Device Tree due to any error,
resource_list_first_type() will return NULL and
pci_parse_request_of_pci_ranges() will just emit a warning.
This will cause a NULL pointer dereference.
Fix this bug by adding NULL return check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
---
v1->v2: Add return code processing as suggested by Bjorn
v2->v3: Return -ENODEV instead of -EINVAL as suggested by Manivannan

 drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 844de4418724..381f7b2b74ca 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
 	} while (val & DBI_CS2);
 }
 
-static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
+static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	u32 val;
 	u32 num_viewport = ks_pcie->num_viewport;
 	struct dw_pcie *pci = ks_pcie->pci;
 	struct dw_pcie_rp *pp = &pci->pp;
-	u64 start, end;
+	struct resource_entry *ft;
 	struct resource *mem;
+	u64 start, end;
 	int i;
 
-	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
+	ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+	if (!ft)
+		return -ENODEV;
+
+	mem = ft->res;
 	start = mem->start;
 	end = mem->end;
 
@@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	ks_pcie_clear_dbi_mode(ks_pcie);
 
 	if (ks_pcie->is_am6)
-		return;
+		return 0;
 
 	val = ilog2(OB_WIN_SIZE);
 	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
@@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
 	val |= OB_XLAT_EN_VAL;
 	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
+
+	return 0;
 }
 
 static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
@@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 		return ret;
 
 	ks_pcie_stop_link(pci);
-	ks_pcie_setup_rc_app_regs(ks_pcie);
+	ret = ks_pcie_setup_rc_app_regs(ks_pcie);
+	if (ret)
+		return ret;
+
 	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
 			pci->dbi_base + PCI_IO_BASE);
 
-- 
2.30.2

Re: [PATCH v3] PCI: dwc: keystone: Fix NULL pointer dereference in case of DT error in ks_pcie_setup_rc_app_regs()

[Manivannan Sadhasivam]

On Fri, May 03, 2024 at 03:57:26PM +0300, Aleksandr Mishin wrote:
> If IORESOURCE_MEM is not provided in Device Tree due to any error,
> resource_list_first_type() will return NULL and
> pci_parse_request_of_pci_ranges() will just emit a warning.
> This will cause a NULL pointer dereference.
> Fix this bug by adding NULL return check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>

One nitpick below. With that addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
> v1->v2: Add return code processing as suggested by Bjorn
> v2->v3: Return -ENODEV instead of -EINVAL as suggested by Manivannan
> 
>  drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..381f7b2b74ca 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
>  	} while (val & DBI_CS2);
>  }
>  
> -static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
> +static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  {
>  	u32 val;
>  	u32 num_viewport = ks_pcie->num_viewport;
>  	struct dw_pcie *pci = ks_pcie->pci;
>  	struct dw_pcie_rp *pp = &pci->pp;
> -	u64 start, end;
> +	struct resource_entry *ft;

s/ft/entry

- Mani

>  	struct resource *mem;
> +	u64 start, end;
>  	int i;
>  
> -	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
> +	ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if (!ft)
> +		return -ENODEV;
> +
> +	mem = ft->res;
>  	start = mem->start;
>  	end = mem->end;
>  
> @@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	ks_pcie_clear_dbi_mode(ks_pcie);
>  
>  	if (ks_pcie->is_am6)
> -		return;
> +		return 0;
>  
>  	val = ilog2(OB_WIN_SIZE);
>  	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
> @@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
>  	val |= OB_XLAT_EN_VAL;
>  	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
> +
> +	return 0;
>  }
>  
>  static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
> @@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
>  		return ret;
>  
>  	ks_pcie_stop_link(pci);
> -	ks_pcie_setup_rc_app_regs(ks_pcie);
> +	ret = ks_pcie_setup_rc_app_regs(ks_pcie);
> +	if (ret)
> +		return ret;
> +
>  	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
>  			pci->dbi_base + PCI_IO_BASE);
>  
> -- 
> 2.30.2
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH v4] PCI: dwc: keystone: Fix NULL pointer dereference in case of DT error in ks_pcie_setup_rc_app_regs()

[Aleksandr Mishin]

If IORESOURCE_MEM is not provided in Device Tree due to any error,
resource_list_first_type() will return NULL and
pci_parse_request_of_pci_ranges() will just emit a warning.
This will cause a NULL pointer dereference.
Fix this bug by adding NULL return check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources")
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
v1->v2: Add return code processing as suggested by Bjorn
v2->v3: Return -ENODEV instead of -EINVAL as suggested by Manivannan
v3->v4: Change variable name as suggested by Manivannan,
 add "Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>"

 drivers/pci/controller/dwc/pci-keystone.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 844de4418724..67415c1a93ff 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -382,17 +382,22 @@ static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie)
 	} while (val & DBI_CS2);
 }
 
-static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
+static int ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	u32 val;
 	u32 num_viewport = ks_pcie->num_viewport;
 	struct dw_pcie *pci = ks_pcie->pci;
 	struct dw_pcie_rp *pp = &pci->pp;
-	u64 start, end;
+	struct resource_entry *entry;
 	struct resource *mem;
+	u64 start, end;
 	int i;
 
-	mem = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM)->res;
+	entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
+	if (!entry)
+		return -ENODEV;
+
+	mem = entry->res;
 	start = mem->start;
 	end = mem->end;
 
@@ -403,7 +408,7 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	ks_pcie_clear_dbi_mode(ks_pcie);
 
 	if (ks_pcie->is_am6)
-		return;
+		return 0;
 
 	val = ilog2(OB_WIN_SIZE);
 	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
@@ -420,6 +425,8 @@ static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	val = ks_pcie_app_readl(ks_pcie, CMD_STATUS);
 	val |= OB_XLAT_EN_VAL;
 	ks_pcie_app_writel(ks_pcie, CMD_STATUS, val);
+
+	return 0;
 }
 
 static void __iomem *ks_pcie_other_map_bus(struct pci_bus *bus,
@@ -814,7 +821,10 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 		return ret;
 
 	ks_pcie_stop_link(pci);
-	ks_pcie_setup_rc_app_regs(ks_pcie);
+	ret = ks_pcie_setup_rc_app_regs(ks_pcie);
+	if (ret)
+		return ret;
+
 	writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
 			pci->dbi_base + PCI_IO_BASE);
 
-- 
2.30.2

Re: [PATCH v4] PCI: dwc: keystone: Fix NULL pointer dereference in case of DT error in ks_pcie_setup_rc_app_regs()

[Krzysztof Wilczyński]

Hello,

> If IORESOURCE_MEM is not provided in Device Tree due to any error,
> resource_list_first_type() will return NULL and
> pci_parse_request_of_pci_ranges() will just emit a warning.
> This will cause a NULL pointer dereference.
> Fix this bug by adding NULL return check.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.

Applied to controller/dwc, thank you!

[1/1] PCI: dwc: keystone: Fix NULL pointer dereference in case of DT error in ks_pcie_setup_rc_app_regs()
      https://git.kernel.org/pci/pci/c/b1d4d63d52a7

	Krzysztof