Re: [PATCH v2 4/8] clk: clocking-wizard: Add static-config clock provider support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flexible array access violates `__counted_by` bounds check due to uninitialized `num` variable in `clk_wzrd_probe`.
--

--- Patch [4]: [PATCH v2 4/8] clk: clocking-wizard: Add static-config clock provider support ---
commit 0905a91a49c4de73e34946b056e201766ae078dd
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Add static-config clock provider support

This commit adds support for static-config mode in the clocking wizard.
It bypasses AXI register mapping and advertises output clocks as
clk_fixed_factor children of clk_in1 using synthesized ratios. It also
adds a 10 us delay in the enable hook to allow the clock to settle.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index ffc78c90bee6f..5470a717fccc8 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
[ ... ]
> @@ -1224,6 +1314,51 @@ static int clk_wzrd_probe(struct platform_device *pdev)
[ ... ]
> +	} else {
> +		u32 mul_div[WZRD_NUM_OUTPUTS * 2];
> +		const struct clk_parent_data parent_data = { .fw_name = "clk_in1" };
> +		int num_elems = nr_outputs * 2;
> +
> +		/*
> +		 * xlnx,clk-mul-div is a uint32-matrix of <mul div> pairs;
> +		 * FDT encodes it as a flat u32 array so we can read it directly.
> +		 */
> +		ret = of_property_read_u32_array(np, "xlnx,clk-mul-div",
> +						 mul_div, num_elems);
> +		if (ret) {
> +			dev_err(&pdev->dev, "xlnx,clk-mul-div missing or invalid\n");
> +			return ret;
> +		}
> +
> +		for (i = 0; i < nr_outputs; i++) {
> +			u32 mul = mul_div[2 * i];
> +			u32 div = mul_div[2 * i + 1];
> +
> +			if (!mul || !div)
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +						     "invalid mul/div for clkout%d\n", i);
> +
> +			clk_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> +						  "%s_out%d", dev_name(&pdev->dev), i);
> +			if (!clk_name)
> +				return -ENOMEM;
> +
> +			hw = clk_wzrd_devm_register_static_fixed_factor(&pdev->dev, clk_name,
> +									&parent_data,
> +									CLK_SET_RATE_PARENT,
> +									mul, div);
> +			if (IS_ERR(hw))
> +				return PTR_ERR(hw);
> +			clk_wzrd->clk_data.hws[i] = hw;

[Severity: High]
Will this trigger a UBSAN bounds-check panic when assigning to hws[i]?

The struct clk_hw_onecell_data uses the __counted_by(num) attribute for
its hws flexible array. Since clk_wzrd->clk_data.num is zero here (from
the initial kzalloc) and is only updated after this loop, does writing
to hws[i] violate the __counted_by bounds check?

Should clk_wzrd->clk_data.num be set to nr_outputs before entering the
loop to prevent a panic during device probe when CONFIG_UBSAN_BOUNDS
is enabled?

It looks like the pre-existing dynamic config path in
clk_wzrd_register_output_clocks() suffers from this same initialization
ordering issue, even though this specific loop is new to this patch.

> +		}
> +
> +		clk_wzrd->clk_data.num = nr_outputs;
> +
> +		ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
> +						  &clk_wzrd->clk_data);
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "unable to register clock provider\n");
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709111403.1579159-1-shubhrajyoti.datta@amd.com?part=4