[PATCH 0/2] PCI: rcar: Misc error path fixes

[PATCH 0/2] PCI: rcar: Misc error path fixes

[Geert Uytterhoeven]

	Hi Simon, Lorenzo, Bjorn,

This patch series fixes two issues in the error path for the R-Car PCIe
host bridge driver.

The first issue is triggered easily by not having a PCIe card inserted,
and may cause a crash.

Thanks!

Geert Uytterhoeven (2):
  PCI: rcar: Fix use-after-free in probe error path
  PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

 drivers/pci/host/pcie-rcar.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/2] PCI: rcar: Fix use-after-free in probe error path

[Geert Uytterhoeven]

If CONFIG_DEBUG_SLAB=y, and no PCIe card is inserted, the kernel crashes
during probe on r8a7791/koelsch:

    rcar-pcie fe000000.pcie: PCIe link down
    Unable to handle kernel paging request at virtual address 6b6b6b6b

(seeing this message requires earlycon and keep_bootcon).

Indeed, pci_free_host_bridge() frees the PCI host bridge, including the
embedded rcar_pcie object, so pci_free_resource_list() must not be
called afterwards.

To fix this, move the call to pci_free_resource_list() up, and update the
label name accordingly.

Fixes: ddd535f1ea3eb27e ("PCI: rcar: Fix memory leak when no PCIe card is inserted")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pci/host/pcie-rcar.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 12796eccb2befd91..52ab3cb0a0bfe065 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -1128,12 +1128,12 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	err = rcar_pcie_get_resources(pcie);
 	if (err < 0) {
 		dev_err(dev, "failed to request resources: %d\n", err);
-		goto err_free_bridge;
+		goto err_free_resource_list;
 	}
 
 	err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
 	if (err)
-		goto err_free_bridge;
+		goto err_free_resource_list;
 
 	pm_runtime_enable(dev);
 	err = pm_runtime_get_sync(dev);
@@ -1176,9 +1176,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 err_pm_disable:
 	pm_runtime_disable(dev);
 
-err_free_bridge:
-	pci_free_host_bridge(bridge);
+err_free_resource_list:
 	pci_free_resource_list(&pcie->resources);
+	pci_free_host_bridge(bridge);
 
 	return err;
 }
-- 
2.7.4

Re: [PATCH 1/2] PCI: rcar: Fix use-after-free in probe error path

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:19AM +0100, Geert Uytterhoeven wrote:
> If CONFIG_DEBUG_SLAB=y, and no PCIe card is inserted, the kernel crashes
> during probe on r8a7791/koelsch:
> 
>     rcar-pcie fe000000.pcie: PCIe link down
>     Unable to handle kernel paging request at virtual address 6b6b6b6b
> 
> (seeing this message requires earlycon and keep_bootcon).
> 
> Indeed, pci_free_host_bridge() frees the PCI host bridge, including the
> embedded rcar_pcie object, so pci_free_resource_list() must not be
> called afterwards.
> 
> To fix this, move the call to pci_free_resource_list() up, and update the
> label name accordingly.
> 
> Fixes: ddd535f1ea3eb27e ("PCI: rcar: Fix memory leak when no PCIe card is inserted")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pci/host/pcie-rcar.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 12796eccb2befd91..52ab3cb0a0bfe065 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1128,12 +1128,12 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	err = rcar_pcie_get_resources(pcie);
>  	if (err < 0) {
>  		dev_err(dev, "failed to request resources: %d\n", err);
> -		goto err_free_bridge;
> +		goto err_free_resource_list;
>  	}
>  
>  	err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>  	if (err)
> -		goto err_free_bridge;
> +		goto err_free_resource_list;
>  
>  	pm_runtime_enable(dev);
>  	err = pm_runtime_get_sync(dev);
> @@ -1176,9 +1176,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  err_pm_disable:
>  	pm_runtime_disable(dev);
>  
> -err_free_bridge:
> -	pci_free_host_bridge(bridge);
> +err_free_resource_list:
>  	pci_free_resource_list(&pcie->resources);
> +	pci_free_host_bridge(bridge);
>  
>  	return err;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH 1/2] PCI: rcar: Fix use-after-free in probe error path

[Bjorn Helgaas]

On Thu, Dec 07, 2017 at 11:15:19AM +0100, Geert Uytterhoeven wrote:
> If CONFIG_DEBUG_SLAB=y, and no PCIe card is inserted, the kernel crashes
> during probe on r8a7791/koelsch:
> 
>     rcar-pcie fe000000.pcie: PCIe link down
>     Unable to handle kernel paging request at virtual address 6b6b6b6b
> 
> (seeing this message requires earlycon and keep_bootcon).
> 
> Indeed, pci_free_host_bridge() frees the PCI host bridge, including the
> embedded rcar_pcie object, so pci_free_resource_list() must not be
> called afterwards.
> 
> To fix this, move the call to pci_free_resource_list() up, and update the
> label name accordingly.
> 
> Fixes: ddd535f1ea3eb27e ("PCI: rcar: Fix memory leak when no PCIe card is inserted")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Applied with Simon's and Lorenzo's acks to for-linus for v4.15, thanks!

I'll leave the second patch up to Lorenzo for v4.16, since it doesn't seem
as critical.

> ---
>  drivers/pci/host/pcie-rcar.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 12796eccb2befd91..52ab3cb0a0bfe065 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1128,12 +1128,12 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	err = rcar_pcie_get_resources(pcie);
>  	if (err < 0) {
>  		dev_err(dev, "failed to request resources: %d\n", err);
> -		goto err_free_bridge;
> +		goto err_free_resource_list;
>  	}
>  
>  	err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>  	if (err)
> -		goto err_free_bridge;
> +		goto err_free_resource_list;
>  
>  	pm_runtime_enable(dev);
>  	err = pm_runtime_get_sync(dev);
> @@ -1176,9 +1176,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  err_pm_disable:
>  	pm_runtime_disable(dev);
>  
> -err_free_bridge:
> -	pci_free_host_bridge(bridge);
> +err_free_resource_list:
>  	pci_free_resource_list(&pcie->resources);
> +	pci_free_host_bridge(bridge);
>  
>  	return err;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH 1/2] PCI: rcar: Fix use-after-free in probe error path

[Simon Horman]

On Tue, Dec 12, 2017 at 01:29:42PM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 07, 2017 at 11:15:19AM +0100, Geert Uytterhoeven wrote:
> > If CONFIG_DEBUG_SLAB=y, and no PCIe card is inserted, the kernel crashes
> > during probe on r8a7791/koelsch:
> > 
> >     rcar-pcie fe000000.pcie: PCIe link down
> >     Unable to handle kernel paging request at virtual address 6b6b6b6b
> > 
> > (seeing this message requires earlycon and keep_bootcon).
> > 
> > Indeed, pci_free_host_bridge() frees the PCI host bridge, including the
> > embedded rcar_pcie object, so pci_free_resource_list() must not be
> > called afterwards.
> > 
> > To fix this, move the call to pci_free_resource_list() up, and update the
> > label name accordingly.
> > 
> > Fixes: ddd535f1ea3eb27e ("PCI: rcar: Fix memory leak when no PCIe card is inserted")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Applied with Simon's and Lorenzo's acks to for-linus for v4.15, thanks!
> 
> I'll leave the second patch up to Lorenzo for v4.16, since it doesn't seem
> as critical.

Thanks Bjorn, that's fine by me.

[PATCH 2/2] PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

[Geert Uytterhoeven]

rcar_pcie_parse_request_of_pci_ranges() can fail and return an error
code, but this is not checked nor handled.

Fix this by adding the missing error handling.

Fixes: 5d2917d469faab72 ("PCI: rcar: Convert to DT resource parsing API")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pci/host/pcie-rcar.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 52ab3cb0a0bfe065..95ca4a1feba4b759 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -1123,7 +1123,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&pcie->resources);
 
-	rcar_pcie_parse_request_of_pci_ranges(pcie);
+	err = rcar_pcie_parse_request_of_pci_ranges(pcie);
+	if (err)
+		goto err_free_bridge;
 
 	err = rcar_pcie_get_resources(pcie);
 	if (err < 0) {
@@ -1178,6 +1180,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 
 err_free_resource_list:
 	pci_free_resource_list(&pcie->resources);
+err_free_bridge:
 	pci_free_host_bridge(bridge);
 
 	return err;
-- 
2.7.4

Re: [PATCH 2/2] PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> rcar_pcie_parse_request_of_pci_ranges() can fail and return an error
> code, but this is not checked nor handled.
> 
> Fix this by adding the missing error handling.
> 
> Fixes: 5d2917d469faab72 ("PCI: rcar: Convert to DT resource parsing API")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pci/host/pcie-rcar.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 52ab3cb0a0bfe065..95ca4a1feba4b759 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1123,7 +1123,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  	INIT_LIST_HEAD(&pcie->resources);
>  
> -	rcar_pcie_parse_request_of_pci_ranges(pcie);
> +	err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> +	if (err)
> +		goto err_free_bridge;
>  
>  	err = rcar_pcie_get_resources(pcie);
>  	if (err < 0) {
> @@ -1178,6 +1180,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  err_free_resource_list:
>  	pci_free_resource_list(&pcie->resources);
> +err_free_bridge:
>  	pci_free_host_bridge(bridge);
>  
>  	return err;
> -- 
> 2.7.4
> 

Re: [PATCH 2/2] PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> rcar_pcie_parse_request_of_pci_ranges() can fail and return an error
> code, but this is not checked nor handled.
> 
> Fix this by adding the missing error handling.
> 
> Fixes: 5d2917d469faab72 ("PCI: rcar: Convert to DT resource parsing API")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pci/host/pcie-rcar.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied to pci/rcar for v4.16.

Thanks,
Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 52ab3cb0a0bfe065..95ca4a1feba4b759 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1123,7 +1123,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  	INIT_LIST_HEAD(&pcie->resources);
>  
> -	rcar_pcie_parse_request_of_pci_ranges(pcie);
> +	err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> +	if (err)
> +		goto err_free_bridge;
>  
>  	err = rcar_pcie_get_resources(pcie);
>  	if (err < 0) {
> @@ -1178,6 +1180,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  err_free_resource_list:
>  	pci_free_resource_list(&pcie->resources);
> +err_free_bridge:
>  	pci_free_host_bridge(bridge);
>  
>  	return err;
> -- 
> 2.7.4
> 

Re: [PATCH 0/2] PCI: rcar: Misc error path fixes

[Simon Horman]

On Thu, Dec 07, 2017 at 11:15:18AM +0100, Geert Uytterhoeven wrote:
> 	Hi Simon, Lorenzo, Bjorn,
> 
> This patch series fixes two issues in the error path for the R-Car PCIe
> host bridge driver.
> 
> The first issue is triggered easily by not having a PCIe card inserted,
> and may cause a crash.
> 
> Thanks!

Acked-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 0/2] PCI: rcar: Misc error path fixes

[Bjorn Helgaas]

On Thu, Dec 07, 2017 at 11:15:18AM +0100, Geert Uytterhoeven wrote:
> 	Hi Simon, Lorenzo, Bjorn,
> 
> This patch series fixes two issues in the error path for the R-Car PCIe
> host bridge driver.
> 
> The first issue is triggered easily by not having a PCIe card inserted,
> and may cause a crash.
> 
> Thanks!
> 
> Geert Uytterhoeven (2):
>   PCI: rcar: Fix use-after-free in probe error path
>   PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures
> 
>  drivers/pci/host/pcie-rcar.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

The first fixes ddd535f1ea3eb27e, which appeared in v4.14-rc1.  The
second fixes 5d2917d469faab72, which appeared in v4.5-rc1.

After the merge window I normally just pull in critical fixes and
fixes to things we merged during the window.

I think the first makes sense for v4.15 because it can cause a crash
with no obvious way to debug it.  The second one feels like v4.16
material to me.

Thoughts?  We're trying to sort out how to handle this sort of thing
between Lorenzo and myself, so I apologize for the confusion here.

Bjorn

Re: [PATCH 0/2] PCI: rcar: Misc error path fixes

[Geert Uytterhoeven]

Hi Bjorn,

On Tue, Dec 12, 2017 at 6:16 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Thu, Dec 07, 2017 at 11:15:18AM +0100, Geert Uytterhoeven wrote:
>> This patch series fixes two issues in the error path for the R-Car PCIe
>> host bridge driver.
>>
>> The first issue is triggered easily by not having a PCIe card inserted,
>> and may cause a crash.
>>
>> Thanks!
>>
>> Geert Uytterhoeven (2):
>>   PCI: rcar: Fix use-after-free in probe error path
>>   PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures
>>
>>  drivers/pci/host/pcie-rcar.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> The first fixes ddd535f1ea3eb27e, which appeared in v4.14-rc1.  The
> second fixes 5d2917d469faab72, which appeared in v4.5-rc1.
>
> After the merge window I normally just pull in critical fixes and
> fixes to things we merged during the window.
>
> I think the first makes sense for v4.15 because it can cause a crash
> with no obvious way to debug it.  The second one feels like v4.16
> material to me.
>
> Thoughts?  We're trying to sort out how to handle this sort of thing
> between Lorenzo and myself, so I apologize for the confusion here.

Sounds fine to me.
The first one happens all the time if no PCIe card is present.
The second one is non-critical.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] PCI: rcar: Misc error path fixes

[Lorenzo Pieralisi]

On Tue, Dec 12, 2017 at 11:16:58AM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 07, 2017 at 11:15:18AM +0100, Geert Uytterhoeven wrote:
> > 	Hi Simon, Lorenzo, Bjorn,
> > 
> > This patch series fixes two issues in the error path for the R-Car PCIe
> > host bridge driver.
> > 
> > The first issue is triggered easily by not having a PCIe card inserted,
> > and may cause a crash.
> > 
> > Thanks!
> > 
> > Geert Uytterhoeven (2):
> >   PCI: rcar: Fix use-after-free in probe error path
> >   PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures
> > 
> >  drivers/pci/host/pcie-rcar.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> The first fixes ddd535f1ea3eb27e, which appeared in v4.14-rc1.  The
> second fixes 5d2917d469faab72, which appeared in v4.5-rc1.
> 
> After the merge window I normally just pull in critical fixes and
> fixes to things we merged during the window.
> 
> I think the first makes sense for v4.15 because it can cause a crash
> with no obvious way to debug it.  The second one feels like v4.16
> material to me.
> 
> Thoughts?  We're trying to sort out how to handle this sort of thing
> between Lorenzo and myself, so I apologize for the confusion here.

This sounds absolutely reasonable for me.

Thanks !
Lorenzo