Re: [PATCH] clk: eswin: Zero-initialize stack-allocated clk_init_data

[Xuyang Dong]

> 
> eswin_clk_register_pll() and eswin_register_clkdiv() declare a struct
> clk_init_data on the stack and only initialize some of its fields
> (parent_data respectively parent_hws). clk_core_populate_parent_map()
> checks parent_names first and parent_data second before falling back
> to parent_hws, so leftover stack garbage in the uninitialized fields
> hijacks parent resolution and the clk core dereferences a bogus
> pointer:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 000000000000000c
>   Oops [#1]
>   epc : __clk_register+0x31a/0x7f0
>   [<ffffffff805dc774>] __clk_register+0x31a/0x7f0
>   [<ffffffff805dcd76>] devm_clk_hw_register+0x2a/0x94
>   [<ffffffff805e319a>] eswin_register_clkdiv+0x80/0xd0
>   [<ffffffff805e34a0>] eswin_clk_register_clks+0x162/0x1a0
>   [<ffffffff805e3736>] eic7700_clk_probe+0x146/0x180
>   [<ffffffff8065d23c>] platform_probe+0x3c/0x7a
> 
> Observed on EIC7700 hardware (with the driver backported to a 6.17
> tree); whether the bug triggers depends entirely on what the stack
> happens to contain when the registration helpers run.
> 
> Zero-initialize both structures.
> 
> Fixes: cd44f127c1d4 ("clk: eswin: Add eic7700 clock driver")
> Signed-off-by: Kostas Damaskinakis <kostas.damaskinakis@xxxxxxxxx>
> ---
>  drivers/clk/eswin/clk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/eswin/clk.c b/drivers/clk/eswin/clk.c
> index e09a52cc3..79d1e4c5e 100644
> --- a/drivers/clk/eswin/clk.c
> +++ b/drivers/clk/eswin/clk.c
> @@ -204,7 +204,7 @@ int eswin_clk_register_pll(struct device *dev, struct eswin_pll_clock *clks,
>  			   int nums, struct eswin_clock_data *data)
>  {
>  	struct eswin_clk_pll *p_clk = NULL;
> -	struct clk_init_data init;
> +	struct clk_init_data init = {};
>  	struct clk_hw *clk_hw;
>  	int i, ret;
>  
> @@ -419,7 +419,7 @@ struct clk_hw *eswin_register_clkdiv(struct device *dev, unsigned int id,
>  				     unsigned long priv_flag, spinlock_t *lock)
>  {
>  	struct eswin_divider_clock *dclk;
> -	struct clk_init_data init;
> +	struct clk_init_data init = {};
>  	struct clk_hw *clk_hw;
>  	int ret;
>  

Acked-by: Xuyang Dong <dongxuyang@xxxxxxxxxxxxxxxxxx>

Thanks,
Xuyang