[PATCH] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[PATCH] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Wen Yang]

The cpu variable is still being used in the of_get_property() call
after the of_node_put() call, which may result in use-after-free.

Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/cpufreq/pasemi-cpufreq.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index 6b1e4ab..d2dd47b 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -132,7 +132,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 	cpu = of_get_cpu_node(policy->cpu, NULL);
 
-	of_node_put(cpu);
 	if (!cpu)
 		goto out;
 
@@ -141,15 +140,15 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dn = of_find_compatible_node(NULL, NULL,
 					     "pasemi,pwrficient-sdc");
 	if (!dn)
-		goto out;
+		goto out_put_cpu_node;
 	err = of_address_to_resource(dn, 0, &res);
 	of_node_put(dn);
 	if (err)
-		goto out;
+		goto out_put_cpu_node;
 	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
 	if (!sdcasr_mapbase) {
 		err = -EINVAL;
-		goto out;
+		goto out_put_cpu_node;
 	}
 
 	dn = of_find_compatible_node(NULL, NULL, "1682m-gizmo");
@@ -177,6 +176,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		err = -EINVAL;
 		goto out_unmap_sdcpwr;
 	}
+	of_node_put(cpu);
 
 	/* we need the freq in kHz */
 	max_freq = *max_freqp / 1000;
@@ -203,6 +203,8 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 out_unmap_sdcasr:
 	iounmap(sdcasr_mapbase);
+out_put_cpu_node:
+	of_node_put(cpu);
 out:
 	return err;
 }
-- 
2.9.5

[PATCH] crypto: crypto4xx: fix a potential double free in ppc4xx_trng_probe

[Wen Yang]

There is a possible double free issue in ppc4xx_trng_probe():

85:	dev->trng_base = of_iomap(trng, 0);
86:	of_node_put(trng);          ---> released here
87:	if (!dev->trng_base)
88:		goto err_out;
...
110:	ierr_out:
111:		of_node_put(trng);  ---> double released here
...

This issue was detected by using the Coccinelle software.
We fix it by removing the unnecessary of_node_put().

Fixes: 5343e674f32 ("crypto4xx: integrate ppc4xx-rng into crypto4xx")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Allison Randal <allison@lohutok.net>
Cc: Armijn Hemel <armijn@tjaldur.nl>
Cc: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/amcc/crypto4xx_trng.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/amcc/crypto4xx_trng.c b/drivers/crypto/amcc/crypto4xx_trng.c
index 02a6bed3..f10a87e 100644
--- a/drivers/crypto/amcc/crypto4xx_trng.c
+++ b/drivers/crypto/amcc/crypto4xx_trng.c
@@ -108,7 +108,6 @@ void ppc4xx_trng_probe(struct crypto4xx_core_device *core_dev)
 	return;
 
 err_out:
-	of_node_put(trng);
 	iounmap(dev->trng_base);
 	kfree(rng);
 	dev->trng_base = NULL;
-- 
2.9.5

[PATCH] irqchip: renesas-rza1: fix an use-after-free in rza1_irqc_probe()

[Wen Yang]

The gic_node is still being used in the rza1_irqc_parse_map() call
after the of_node_put() call, which may result in use-after-free.

Fixes: a644ccb819bc ("irqchip: Add Renesas RZ/A1 Interrupt Controller driver")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Chris Brandt <chris.brandt@renesas.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: linux-kernel@vger.kernel.org
---
 drivers/irqchip/irq-renesas-rza1.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-rza1.c b/drivers/irqchip/irq-renesas-rza1.c
index b1f19b21..b0d46ac 100644
--- a/drivers/irqchip/irq-renesas-rza1.c
+++ b/drivers/irqchip/irq-renesas-rza1.c
@@ -208,20 +208,19 @@ static int rza1_irqc_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->base);
 
 	gic_node = of_irq_find_parent(np);
-	if (gic_node) {
+	if (gic_node)
 		parent = irq_find_host(gic_node);
-		of_node_put(gic_node);
-	}
 
 	if (!parent) {
 		dev_err(dev, "cannot find parent domain\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_put_node;
 	}
 
 	ret = rza1_irqc_parse_map(priv, gic_node);
 	if (ret) {
 		dev_err(dev, "cannot parse %s: %d\n", "interrupt-map", ret);
-		return ret;
+		goto out_put_node;
 	}
 
 	priv->chip.name = "rza1-irqc",
@@ -237,10 +236,12 @@ static int rza1_irqc_probe(struct platform_device *pdev)
 						    priv);
 	if (!priv->irq_domain) {
 		dev_err(dev, "cannot initialize irq domain\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
 	}
 
-	return 0;
+out_put_node:
+	of_node_put(gic_node);
+	return ret;
 }
 
 static int rza1_irqc_remove(struct platform_device *pdev)
-- 
2.9.5

[PATCH] phy: ti: am654-serdes: fix an use-after-free in serdes_am654_clk_register()

[Wen Yang]

The regmap_node variable is still being used in the syscon_node_to_regmap()
call after the of_node_put() call, which may result in use-after-free.

Fixes: 71e2f5c5c224 ("phy: ti: Add a new SERDES driver for TI's AM654x SoC")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Roger Quadros <rogerq@ti.com>
Cc: linux-kernel@vger.kernel.org
---
 drivers/phy/ti/phy-am654-serdes.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/phy/ti/phy-am654-serdes.c b/drivers/phy/ti/phy-am654-serdes.c
index f8edd08..f14f1f0 100644
--- a/drivers/phy/ti/phy-am654-serdes.c
+++ b/drivers/phy/ti/phy-am654-serdes.c
@@ -405,6 +405,7 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
 	const __be32 *addr;
 	unsigned int reg;
 	struct clk *clk;
+	int ret = 0;
 
 	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux)
@@ -413,34 +414,40 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
 	init = &mux->clk_data;
 
 	regmap_node = of_parse_phandle(node, "ti,serdes-clk", 0);
-	of_node_put(regmap_node);
 	if (!regmap_node) {
 		dev_err(dev, "Fail to get serdes-clk node\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_put_node;
 	}
 
 	regmap = syscon_node_to_regmap(regmap_node->parent);
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "Fail to get Syscon regmap\n");
-		return PTR_ERR(regmap);
+		ret = PTR_ERR(regmap);
+		goto out_put_node;
 	}
 
 	num_parents = of_clk_get_parent_count(node);
 	if (num_parents < 2) {
 		dev_err(dev, "SERDES clock must have parents\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_put_node;
 	}
 
 	parent_names = devm_kzalloc(dev, (sizeof(char *) * num_parents),
 				    GFP_KERNEL);
-	if (!parent_names)
-		return -ENOMEM;
+	if (!parent_names) {
+		ret = -ENOMEM;
+		goto out_put_node;
+	}
 
 	of_clk_parent_fill(node, parent_names, num_parents);
 
 	addr = of_get_address(regmap_node, 0, NULL, NULL);
-	if (!addr)
-		return -EINVAL;
+	if (!addr) {
+		ret = -EINVAL;
+		goto out_put_node;
+	}
 
 	reg = be32_to_cpu(*addr);
 
@@ -456,12 +463,16 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
 	mux->hw.init = init;
 
 	clk = devm_clk_register(dev, &mux->hw);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto out_put_node;
+	}
 
 	am654_phy->clks[clock_num] = clk;
 
-	return 0;
+out_put_node:
+	of_node_put(regmap_node);
+	return ret;
 }
 
 static const struct of_device_id serdes_am654_id_table[] = {
-- 
2.9.5

Re: [PATCH] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()

[Viresh Kumar]

On 08-07-19, 14:19, Wen Yang wrote:
> The cpu variable is still being used in the of_get_property() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/cpufreq/pasemi-cpufreq.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

I will suggest some changes here.

> diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> index 6b1e4ab..d2dd47b 100644
> --- a/drivers/cpufreq/pasemi-cpufreq.c
> +++ b/drivers/cpufreq/pasemi-cpufreq.c
> @@ -132,7 +132,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)

Don't initialize "err" anymore.

>  	cpu = of_get_cpu_node(policy->cpu, NULL);
>  
> -	of_node_put(cpu);
>  	if (!cpu)
>  		goto out;

Do return -ENODEV; here.

>  
> @@ -141,15 +140,15 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  		dn = of_find_compatible_node(NULL, NULL,
>  					     "pasemi,pwrficient-sdc");
>  	if (!dn)
> -		goto out;
> +		goto out_put_cpu_node;
>  	err = of_address_to_resource(dn, 0, &res);
>  	of_node_put(dn);
>  	if (err)
> -		goto out;
> +		goto out_put_cpu_node;
>  	sdcasr_mapbase = ioremap(res.start + SDCASR_OFFSET, 0x2000);
>  	if (!sdcasr_mapbase) {
>  		err = -EINVAL;
> -		goto out;
> +		goto out_put_cpu_node;
>  	}

Don't do above changes.

>  
>  	dn = of_find_compatible_node(NULL, NULL, "1682m-gizmo");
> @@ -177,6 +176,7 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  		err = -EINVAL;
>  		goto out_unmap_sdcpwr;
>  	}
> +	of_node_put(cpu);
>  
>  	/* we need the freq in kHz */
>  	max_freq = *max_freqp / 1000;
> @@ -203,6 +203,8 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  
>  out_unmap_sdcasr:
>  	iounmap(sdcasr_mapbase);
> +out_put_cpu_node:

Don't add this label, instead use "out" for also having the below
code.

> +	of_node_put(cpu);
>  out:
>  	return err;
>  }
> -- 
> 2.9.5

-- 
viresh

Re: [PATCH] crypto: crypto4xx: fix a potential double free in ppc4xx_trng_probe

[Julia Lawall]

On Mon, 8 Jul 2019, Wen Yang wrote:

> There is a possible double free issue in ppc4xx_trng_probe():
>
> 85:	dev->trng_base = of_iomap(trng, 0);
> 86:	of_node_put(trng);          ---> released here
> 87:	if (!dev->trng_base)
> 88:		goto err_out;
> ...
> 110:	ierr_out:
> 111:		of_node_put(trng);  ---> double released here
> ...
>
> This issue was detected by using the Coccinelle software.
> We fix it by removing the unnecessary of_node_put().
>
> Fixes: 5343e674f32 ("crypto4xx: integrate ppc4xx-rng into crypto4xx")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Armijn Hemel <armijn@tjaldur.nl>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

Acked-by: Julia Lawall <julia.lawall@lip6.fr>


> ---
>  drivers/crypto/amcc/crypto4xx_trng.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/crypto/amcc/crypto4xx_trng.c b/drivers/crypto/amcc/crypto4xx_trng.c
> index 02a6bed3..f10a87e 100644
> --- a/drivers/crypto/amcc/crypto4xx_trng.c
> +++ b/drivers/crypto/amcc/crypto4xx_trng.c
> @@ -108,7 +108,6 @@ void ppc4xx_trng_probe(struct crypto4xx_core_device *core_dev)
>  	return;
>
>  err_out:
> -	of_node_put(trng);
>  	iounmap(dev->trng_base);
>  	kfree(rng);
>  	dev->trng_base = NULL;
> --
> 2.9.5
>
>

Re: [PATCH] irqchip: renesas-rza1: fix an use-after-free in rza1_irqc_probe()

[Geert Uytterhoeven]

Hi Wen,

On Mon, Jul 8, 2019 at 8:22 AM Wen Yang <wen.yang99@zte.com.cn> wrote:
> The gic_node is still being used in the rza1_irqc_parse_map() call
> after the of_node_put() call, which may result in use-after-free.

Thanks! This use was added in v3, but I forgot to move the of_node_put() call.

> Fixes: a644ccb819bc ("irqchip: Add Renesas RZ/A1 Interrupt Controller driver")

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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: Coccinelle: Checking the deletion of duplicate of_node_put() calls with SmPL

[Markus Elfring]

> 110:	ierr_out:

> 111:		of_node_put(trng);  ---> double released here

> ...


>
> This issue was detected by using the Coccinelle software.

Such a detection of a questionable source code place can be nice and helpful.

I constructed another script variant for the semantic patch language.

@deletion@
expression x;
identifier target;
@@
 of_node_put(x);
 if (...)
    goto target;
 ... when any
 target:
-of_node_put(x);


I observe then that this adjustment approach can generate the desired patch
for a source code extract.

elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch ../janitor/delete_duplicate_of_node_put1.cocci crypto4xx_trng-excerpt1.c

…
-	of_node_put(trng);

…


But I wonder at the moment why it does not work (as expected) for the original
complete source file.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/amcc/crypto4xx_trng.c?id=5ad18b2e60b75c7297a998dea702451d33a052ed#n71
https://elixir.bootlin.com/linux/v5.2/source/drivers/crypto/amcc/crypto4xx_trng.c#L71

I am curious on further software development ideas.

Regards,
Markus

[tip:irq/urgent] irqchip/renesas-rza1: Prevent use-after-free in rza1_irqc_probe()

[tip-bot for Wen Yang]

Commit-ID:  7c8e90ddf02f139a90bc29c04302e9914818f0c8
Gitweb:     https://git.kernel.org/tip/7c8e90ddf02f139a90bc29c04302e9914818f0c8
Author:     Wen Yang <wen.yang99@zte.com.cn>
AuthorDate: Mon, 8 Jul 2019 14:19:04 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 9 Jul 2019 14:53:50 +0200

irqchip/renesas-rza1: Prevent use-after-free in rza1_irqc_probe()

The gic_node is still being used in the rza1_irqc_parse_map() call
after the of_node_put() call, which may result in use-after-free.

Fixes: a644ccb819bc ("irqchip: Add Renesas RZ/A1 Interrupt Controller driver")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lkml.kernel.org/r/1562566745-7447-3-git-send-email-wen.yang99@zte.com.cn
---
 drivers/irqchip/irq-renesas-rza1.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-rza1.c b/drivers/irqchip/irq-renesas-rza1.c
index b1f19b210190..b0d46ac42b89 100644
--- a/drivers/irqchip/irq-renesas-rza1.c
+++ b/drivers/irqchip/irq-renesas-rza1.c
@@ -208,20 +208,19 @@ static int rza1_irqc_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->base);
 
 	gic_node = of_irq_find_parent(np);
-	if (gic_node) {
+	if (gic_node)
 		parent = irq_find_host(gic_node);
-		of_node_put(gic_node);
-	}
 
 	if (!parent) {
 		dev_err(dev, "cannot find parent domain\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_put_node;
 	}
 
 	ret = rza1_irqc_parse_map(priv, gic_node);
 	if (ret) {
 		dev_err(dev, "cannot parse %s: %d\n", "interrupt-map", ret);
-		return ret;
+		goto out_put_node;
 	}
 
 	priv->chip.name = "rza1-irqc",
@@ -237,10 +236,12 @@ static int rza1_irqc_probe(struct platform_device *pdev)
 						    priv);
 	if (!priv->irq_domain) {
 		dev_err(dev, "cannot initialize irq domain\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
 	}
 
-	return 0;
+out_put_node:
+	of_node_put(gic_node);
+	return ret;
 }
 
 static int rza1_irqc_remove(struct platform_device *pdev)

Re: Coccinelle: Checking the deletion of duplicate of_node_put() calls with SmPL

[Markus Elfring]

> But I wonder at the moment why it does not work (as expected) for the original
> complete source file.

I discovered that a diff hunk (or usable patch?) is generated
if the return statement is deleted (or commented out) before the jump label
which refers to a potentially unwanted function call at the mentioned place.
How will the support evolve for automatic adjustment of such source code
combinations by the semantic patch language (Coccinelle software)?

Regards,
Markus

Re: [PATCH] crypto: crypto4xx: fix a potential double free in ppc4xx_trng_probe

[Herbert Xu]

On Mon, Jul 08, 2019 at 02:19:03PM +0800, Wen Yang wrote:
> There is a possible double free issue in ppc4xx_trng_probe():
> 
> 85:	dev->trng_base = of_iomap(trng, 0);
> 86:	of_node_put(trng);          ---> released here
> 87:	if (!dev->trng_base)
> 88:		goto err_out;
> ...
> 110:	ierr_out:
> 111:		of_node_put(trng);  ---> double released here
> ...
> 
> This issue was detected by using the Coccinelle software.
> We fix it by removing the unnecessary of_node_put().
> 
> Fixes: 5343e674f32 ("crypto4xx: integrate ppc4xx-rng into crypto4xx")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Armijn Hemel <armijn@tjaldur.nl>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/crypto/amcc/crypto4xx_trng.c | 1 -
>  1 file changed, 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] irqchip: renesas-rza1: fix an use-after-free in rza1_irqc_probe()

[Marc Zyngier]

On Mon, 08 Jul 2019 07:19:04 +0100,
Wen Yang <wen.yang99@zte.com.cn> wrote:
> 
> The gic_node is still being used in the rza1_irqc_parse_map() call
> after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: a644ccb819bc ("irqchip: Add Renesas RZ/A1 Interrupt Controller driver")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Chris Brandt <chris.brandt@renesas.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/irqchip/irq-renesas-rza1.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-renesas-rza1.c b/drivers/irqchip/irq-renesas-rza1.c
> index b1f19b21..b0d46ac 100644
> --- a/drivers/irqchip/irq-renesas-rza1.c
> +++ b/drivers/irqchip/irq-renesas-rza1.c
> @@ -208,20 +208,19 @@ static int rza1_irqc_probe(struct platform_device *pdev)
>  		return PTR_ERR(priv->base);
>  
>  	gic_node = of_irq_find_parent(np);
> -	if (gic_node) {
> +	if (gic_node)
>  		parent = irq_find_host(gic_node);
> -		of_node_put(gic_node);
> -	}
>  
>  	if (!parent) {
>  		dev_err(dev, "cannot find parent domain\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_put_node;
>  	}
>  
>  	ret = rza1_irqc_parse_map(priv, gic_node);
>  	if (ret) {
>  		dev_err(dev, "cannot parse %s: %d\n", "interrupt-map", ret);
> -		return ret;
> +		goto out_put_node;
>  	}
>  
>  	priv->chip.name = "rza1-irqc",
> @@ -237,10 +236,12 @@ static int rza1_irqc_probe(struct platform_device *pdev)
>  						    priv);
>  	if (!priv->irq_domain) {
>  		dev_err(dev, "cannot initialize irq domain\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
>  	}
>  
> -	return 0;
> +out_put_node:
> +	of_node_put(gic_node);
> +	return ret;
>  }
>  
>  static int rza1_irqc_remove(struct platform_device *pdev)
> -- 
> 2.9.5
> 

Applied, thanks.

	M.

-- 
Jazz is not dead, it just smells funny.

Re: [PATCH] phy: ti: am654-serdes: fix an use-after-free in serdes_am654_clk_register()

[Roger Quadros]

On 08/07/2019 09:19, Wen Yang wrote:
> The regmap_node variable is still being used in the syscon_node_to_regmap()
> call after the of_node_put() call, which may result in use-after-free.
> 
> Fixes: 71e2f5c5c224 ("phy: ti: Add a new SERDES driver for TI's AM654x SoC")
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Roger Quadros <rogerq@ti.com>
> Cc: linux-kernel@vger.kernel.org

Reviewed-by: Roger Quadros <rogerq@ti.com>

> ---
>  drivers/phy/ti/phy-am654-serdes.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-am654-serdes.c b/drivers/phy/ti/phy-am654-serdes.c
> index f8edd08..f14f1f0 100644
> --- a/drivers/phy/ti/phy-am654-serdes.c
> +++ b/drivers/phy/ti/phy-am654-serdes.c
> @@ -405,6 +405,7 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
>  	const __be32 *addr;
>  	unsigned int reg;
>  	struct clk *clk;
> +	int ret = 0;
>  
>  	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
>  	if (!mux)
> @@ -413,34 +414,40 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
>  	init = &mux->clk_data;
>  
>  	regmap_node = of_parse_phandle(node, "ti,serdes-clk", 0);
> -	of_node_put(regmap_node);
>  	if (!regmap_node) {
>  		dev_err(dev, "Fail to get serdes-clk node\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto out_put_node;
>  	}
>  
>  	regmap = syscon_node_to_regmap(regmap_node->parent);
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "Fail to get Syscon regmap\n");
> -		return PTR_ERR(regmap);
> +		ret = PTR_ERR(regmap);
> +		goto out_put_node;
>  	}
>  
>  	num_parents = of_clk_get_parent_count(node);
>  	if (num_parents < 2) {
>  		dev_err(dev, "SERDES clock must have parents\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_put_node;
>  	}
>  
>  	parent_names = devm_kzalloc(dev, (sizeof(char *) * num_parents),
>  				    GFP_KERNEL);
> -	if (!parent_names)
> -		return -ENOMEM;
> +	if (!parent_names) {
> +		ret = -ENOMEM;
> +		goto out_put_node;
> +	}
>  
>  	of_clk_parent_fill(node, parent_names, num_parents);
>  
>  	addr = of_get_address(regmap_node, 0, NULL, NULL);
> -	if (!addr)
> -		return -EINVAL;
> +	if (!addr) {
> +		ret = -EINVAL;
> +		goto out_put_node;
> +	}
>  
>  	reg = be32_to_cpu(*addr);
>  
> @@ -456,12 +463,16 @@ static int serdes_am654_clk_register(struct serdes_am654 *am654_phy,
>  	mux->hw.init = init;
>  
>  	clk = devm_clk_register(dev, &mux->hw);
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		goto out_put_node;
> +	}
>  
>  	am654_phy->clks[clock_num] = clk;
>  
> -	return 0;
> +out_put_node:
> +	of_node_put(regmap_node);
> +	return ret;
>  }
>  
>  static const struct of_device_id serdes_am654_id_table[] = {
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki