Re: [PATCH v1] clk: mux: check value returned from clk_mux_val_to_index()

[Sean Anderson <seanga2@gmail.com>]

On 9/5/23 13:33, Maksim Kiselev wrote:
> The clk_mux_val_to_index() may return -EINVAL. In this case
> clk_mux_get_parent() will return a wrong parent index and
> this will lead to out of bounds error.
> 
> E.g. when we register mux clk:
> 	ret = clk_register(clk, UBOOT_DM_CLK_CCF_MUX, name,
> 			   parent_names[clk_mux_get_parent(clk)]);
> 
> Let's add a check on the return value of clk_mux_val_to_index() and if
> it's negative, set parent index to 0.
> 
> Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
> ---
>   drivers/clk/clk-mux.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 017f25f7a5..00fe916171 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -88,6 +88,7 @@ unsigned int clk_mux_index_to_val(u32 *table, unsigned int flags, u8 index)
>   u8 clk_mux_get_parent(struct clk *clk)
>   {
>   	struct clk_mux *mux = to_clk_mux(clk);
> +	int index;
>   	u32 val;
>   
>   #if IS_ENABLED(CONFIG_SANDBOX_CLK_CCF)
> @@ -98,7 +99,13 @@ u8 clk_mux_get_parent(struct clk *clk)
>   	val >>= mux->shift;
>   	val &= mux->mask;
>   
> -	return clk_mux_val_to_index(clk, mux->table, mux->flags, val);
> +	index = clk_mux_val_to_index(clk, mux->table, mux->flags, val);
> +	if (index < 0) {
> +		log_err("Could not fetch index\n");
> +		index = 0;
> +	}
> +
> +	return index;
>   }
>   
>   static int clk_fetch_parent_index(struct clk *clk,

Reviewed-by: Sean Anderson <seanga2@gmail.com>

But can you add something to the commit (or error) message about
how this indicates a bug in the driver (e.g. there aren't enough
parents to match the hardware).

--Sean