Re: [RFC PATCH 2/4] clk: fix clk not being unlinked from consumers list

[Stephen Boyd <sboyd@kernel.org>]

Quoting Nuno Sá (2022-04-07 06:30:34)
> When a clk_hw is resgistered we add a struct clk handle to it's

s/resgistered/registered/

> consumers list.

Please add that the clk handle is created in __clk_register() per the
alloc_clk() call.


> Hence, we need to remove it when unregistering the
> clk_hw. This could actually lead to a use after free if a provider get's

s/get's/gets/

> removed before a consumer. When removing the consumer, __clk_put() is
> called and that will do 'hlist_del(&clk->clks_node)' which will touch in
> already freed memory.

Did this actually happen? I don't see how __clk_put() is called on the
internal hw->clk pointer. This pointer in hw->clk should be removed but
so far we've kept it around and various clk providers have used it. If
we start removing it now I'm not sure it will work because we would
probably expose many dangling pointer problems.

> 
> Fixes: 1df4046a93e08 ("clk: Combine __clk_get() and __clk_create_clk()")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/clk/clk.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed119182aa1b..e82c3ee1da13 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4198,6 +4198,7 @@ void clk_unregister(struct clk *clk)
>                 pr_warn("%s: unregistering protected clock: %s\n",
>                                         __func__, clk->core->name);
>  
> +       clk_core_unlink_consumer(clk);
>         kref_put(&clk->core->ref, __clk_release);
>         free_clk(clk);
>  unlock:
> -- 
> 2.35.1
>