diff mbox series

[1/2] clk: Fix clk_core_get NULL dereference

Message ID 20240302-linux-next-24-03-01-simple-clock-fixes-v1-1-25f348a5982b@linaro.org (mailing list archive)
State Accepted, archived
Headers show
Series clk: Fix a core error path and missing qcom camcc-x1e80100 enum | expand

Commit Message

Bryan O'Donoghue March 2, 2024, 12:52 a.m. UTC
It is possible for clk_core_get to dereference a NULL in the following
sequence:

clk_core_get()
    of_clk_get_hw_from_clkspec()
        __of_clk_get_hw_from_provider()
            __clk_get_hw()

__clk_get_hw() can return NULL which is dereferenced by clk_core_get() at
hw->core.

Prior to commit dde4eff47c82 ("clk: Look for parents with clkdev based
clk_lookups") the check IS_ERR_OR_NULL() was performed which would have
caught the NULL.

Reading the description of this function it talks about returning NULL but
that cannot be so at the moment.

Update the function to check for hw before dereferencing it and return NULL
if hw is NULL.

Fixes: dde4eff47c82 ("clk: Look for parents with clkdev based clk_lookups")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Stephen Boyd March 9, 2024, 12:36 a.m. UTC | #1
Quoting Bryan O'Donoghue (2024-03-01 16:52:14)
> It is possible for clk_core_get to dereference a NULL in the following
> sequence:
> 
> clk_core_get()
>     of_clk_get_hw_from_clkspec()
>         __of_clk_get_hw_from_provider()
>             __clk_get_hw()
> 
> __clk_get_hw() can return NULL which is dereferenced by clk_core_get() at
> hw->core.
> 
> Prior to commit dde4eff47c82 ("clk: Look for parents with clkdev based
> clk_lookups") the check IS_ERR_OR_NULL() was performed which would have
> caught the NULL.
> 
> Reading the description of this function it talks about returning NULL but
> that cannot be so at the moment.
> 
> Update the function to check for hw before dereferencing it and return NULL
> if hw is NULL.
> 
> Fixes: dde4eff47c82 ("clk: Look for parents with clkdev based clk_lookups")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---

I dug through a bunch of old patches and this looks right. I'm not sure
why a provider would ever return NULL though. A NULL pointer means that
the parent is never going to appear so we shouldn't treat this clk as
orphaned in case the clk needs to be clk_get()able and change parents.
This was all part of my plan to introduce a clk_parent_hw() clk op that
returns a pointer and then implement probe defer through clk_get() when
a clk is orphaned. A NULL clk also means a clk is optional and not
there.

Anyway, I've applied this to clk-next. I hope to send out the
clk_parent_hw clk op series soon.
diff mbox series

Patch

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a3bc7fb90d0f..25371c91a58f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -418,6 +418,9 @@  static struct clk_core *clk_core_get(struct clk_core *core, u8 p_index)
 	if (IS_ERR(hw))
 		return ERR_CAST(hw);
 
+	if (!hw)
+		return NULL;
+
 	return hw->core;
 }