Re: [PATCH] clk: hisilicon: clkdivider-hi6220: use kzalloc_flex

[Brian Masney]

Hi Rosen,

On Thu, Mar 26, 2026 at 07:40:32PM -0700, Rosen Penev wrote:
> Combine allocations using kzalloc_flex and a flexible array member.
> 
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  drivers/clk/hisilicon/clkdivider-hi6220.c | 24 ++++++++---------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/clk/hisilicon/clkdivider-hi6220.c b/drivers/clk/hisilicon/clkdivider-hi6220.c
> index 1787ecefe601..14d72fd716c2 100644
> --- a/drivers/clk/hisilicon/clkdivider-hi6220.c
> +++ b/drivers/clk/hisilicon/clkdivider-hi6220.c
> @@ -35,8 +35,8 @@ struct hi6220_clk_divider {
>  	u8		shift;
>  	u8		width;
>  	u32		mask;
> -	const struct clk_div_table *table;
>  	spinlock_t	*lock;
> +	struct clk_div_table table[];
>  };
>  
>  #define to_hi6220_clk_divider(_hw)	\
> @@ -108,24 +108,19 @@ struct clk *hi6220_register_clkdiv(struct device *dev, const char *name,
>  	u32 max_div, min_div;
>  	int i;
>  
> -	/* allocate the divider */
> -	div = kzalloc_obj(*div);
> -	if (!div)
> -		return ERR_PTR(-ENOMEM);
> -
>  	/* Init the divider table */
>  	max_div = div_mask(width) + 1;
>  	min_div = 1;
>  
> -	table = kzalloc_objs(*table, max_div + 1);
> -	if (!table) {
> -		kfree(div);
> +	/* allocate the divider */
> +	div = kzalloc_flex(*div, table, max_div + 1);
> +	if (!div)
>  		return ERR_PTR(-ENOMEM);
> -	}
>  
>  	for (i = 0; i < max_div; i++) {
> -		table[i].div = min_div + i;
> -		table[i].val = table[i].div - 1;
> +		table = &div->table[i];
> +		table->div = min_div + i;
> +		table->val = table[i].div - 1;
                             ^^^^^^^^^^^^^^^^^
Sashiko has a question:
https://sashiko.dev/#/patchset/20260326042317.122536-1-rosenp%40gmail.com

    Does this code read out-of-bounds and write incorrect values to the table?
    
    Because table is reassigned to point to &div->table[i], accessing table[i].div
    in the following line effectively evaluates to div->table[2 * i].div.
    
    For any i > 0, this reads uninitialized memory from further down the array,
    and when 2 * i >= max_div + 1, it reads beyond the newly allocated flex array.
    Could we use table->val = table->div - 1 instead?

Brian

>  	}
>  
>  	init.name = name;
> @@ -141,14 +136,11 @@ struct clk *hi6220_register_clkdiv(struct device *dev, const char *name,
>  	div->mask = mask_bit ? BIT(mask_bit) : 0;
>  	div->lock = lock;
>  	div->hw.init = &init;
> -	div->table = table;
>  
>  	/* register the clock */
>  	clk = clk_register(dev, &div->hw);
> -	if (IS_ERR(clk)) {
> -		kfree(table);
> +	if (IS_ERR(clk))
>  		kfree(div);
> -	}
>  
>  	return clk;
>  }
> -- 
> 2.53.0
>