diff mbox series

clk: renesas: cpg-mssr: Fix use after free if cpg_mssr_common_init() failed

Message ID 1671747798-12935-1-git-send-email-khoroshilov@ispras.ru
State New
Headers
Series clk: renesas: cpg-mssr: Fix use after free if cpg_mssr_common_init() failed |

Commit Message

Alexey Khoroshilov Dec. 22, 2022, 10:23 p.m. UTC 
  If cpg_mssr_common_init() fails after assigning priv to global variable
cpg_mssr_priv, it deallocates priv, but cpg_mssr_priv keeps dangling
pointer that potentially can be used later.

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

Fixes: 1f7db7bbf031 ("clk: renesas: cpg-mssr: Add early clock support")
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Geert Uytterhoeven Dec. 23, 2022, 9:45 a.m. UTC | #1 
Hi Alexey,

On Thu, Dec 22, 2022 at 11:23 PM Alexey Khoroshilov
<khoroshilov@ispras.ru> wrote:
> If cpg_mssr_common_init() fails after assigning priv to global variable
> cpg_mssr_priv, it deallocates priv, but cpg_mssr_priv keeps dangling
> pointer that potentially can be used later.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 1f7db7bbf031 ("clk: renesas: cpg-mssr: Add early clock support")
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>

Thanks for your patch!

> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -1025,6 +1025,7 @@ static int __init cpg_mssr_common_init(struct device *dev,
>         if (priv->base)
>                 iounmap(priv->base);
>         kfree(priv);
> +       cpg_mssr_priv = NULL;

While this is correct, I think it would be better to just postpone
the initial assignment to cpg_mssr_priv until everything in
cpg_mssr_common_init() has succeeded, i.e. just below the
"return 0;" above.

>
>         return error;
>  }

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
diff mbox series

Patch

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index 1a0cdf001b2f..27da127ca4a8 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -1025,6 +1025,7 @@  static int __init cpg_mssr_common_init(struct device *dev,
 	if (priv->base)
 		iounmap(priv->base);
 	kfree(priv);
+	cpg_mssr_priv = NULL;
 
 	return error;
 }