Re: [PATCH v2 4/8] clk: sunxi-ng: a733: Add PLL clocks support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `hws` flexible array in `sun60i_a733_hw_clks` is allocated with insufficient size, leading to an out-of-bounds read in `sunxi_ccu_probe`.
- [Medium] The driver is missing the `MODULE_DEVICE_TABLE` macro, which prevents it from being autoloaded by udev when built as a loadable module.
--

--- Patch [4]: [PATCH v2 4/8] clk: sunxi-ng: a733: Add PLL clocks support ---
commit 803dab2e98d8379ce658bcbb6ef611389e990997
Author: Junhui Liu <junhui.liu@pigmoral.tech>

clk: sunxi-ng: a733: Add PLL clocks support
    
Add PLL clock support for the main CCU of the Allwinner A733 SoC. The
structure is mostly similar to the sun55i, with the addition of a
PLL_REF clock that normalizes the hardware-detected DCXO/hosc frequency
(19.2MHz, 24MHz, or 26MHz) into a consistent 24MHz reference for all
subsequent PLLs.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> new file mode 100644
> index 0000000000000..f80db6ab1a98f
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c

[ ... ]

> +static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
> +	.hws	= {
> +		[CLK_PLL_REF]		= &pll_ref_clk.common.hw,

[ ... ]

> +		[CLK_PLL_DE_4X]		= &pll_de_4x_clk.common.hw,
> +		[CLK_PLL_DE_3X]		= &pll_de_3x_clk.common.hw,
> +	},
> +	.num	= CLK_FANOUT3 + 1,
> +};

[Severity: High]
Since this structure statically initializes a flexible array, the compiler will
only allocate space for the explicitly initialized elements (up to
CLK_PLL_DE_3X, which is 43 elements). However, .num is set to CLK_FANOUT3 + 1.

Will this cause sunxi_ccu_probe() to read out-of-bounds when iterating up to
.num? 

If the out-of-bounds memory is non-NULL, could this lead to a panic when
dereferencing the pointer?

[ ... ]

> +static const struct of_device_id sun60i_a733_ccu_ids[] = {
> +	{ .compatible = "allwinner,sun60i-a733-ccu" },
> +	{ /* sentinel */ }
> +};

[Severity: Medium]
When this driver is built as a loadable module, will it fail to autoload?

It appears to be missing a MODULE_DEVICE_TABLE(of, sun60i_a733_ccu_ids)
declaration to export the alias metadata for udev.

> +
> +static struct platform_driver sun60i_a733_ccu_driver = {
> +	.probe	= sun60i_a733_ccu_probe,
> +	.driver	= {
> +		.name			= "sun60i-a733-ccu",
> +		.suppress_bind_attrs	= true,
> +		.of_match_table		= sun60i_a733_ccu_ids,
> +	},
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-a733-clk-v2-0-974d188cbe0c@pigmoral.tech?part=4