Re: [PATCH v2 13/14] arm64: dts: exynos8855: add cmu_hsi clock DT nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clk: samsung: exynos8855: heap buffer overflow during cmu_hsi registration
- [Low] dt-bindings: clock: samsung,exynos8855: missing samsung,sysreg property in schema
--

commit bbcbd2201fb19f5e477dc2859ff29864327a9378
Author: Raghav Sharma <raghav.s@samsung.com>

arm64: dts: exynos8855: add cmu_hsi clock DT nodes

This commit adds the required dt node for the cmu_hsi block. It also adds
the sysreg_hsi node which contains bits for DRCG and MEMCLK that are used
by the clock driver.

> diff --git a/arch/arm64/boot/dts/exynos/exynos8855.dtsi b/arch/arm64/boot/dts/exynos/exynos8855.dtsi
> index b461de6a75483..3dd7a4c4c609f 100644
> --- a/arch/arm64/boot/dts/exynos/exynos8855.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos8855.dtsi

[ ... ]

> @@ -241,6 +241,27 @@ pinctrl_pericmmc: pinctrl@154f0000 {
>  			interrupts = <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		cmu_hsi: clock-controller@17000000 {

[Severity: High]
Does exposing this node in the device tree trigger a heap buffer overflow
during driver probe?

Looking at the cmu_hsi clock provider in
drivers/clk/samsung/clk-exynos8855.c, the array size for the hardware
pointers is defined as:

#define CLKS_NR_HSI                     (CLK_GOUT_SYSREG_HSI  + 1)

Since CLK_GOUT_SYSREG_HSI evaluates to 4, this allocates an array of 5 elements
during samsung_clk_init(). However,
include/dt-bindings/clock/samsung,exynos8855.h defines higher clock IDs
for this provider:

#define CLK_GOUT_SYSREG_HSI                4
#define CLK_GOUT_UFS_EMBD_ACLK_HSI         5
#define CLK_GOUT_UFS_EMBD_UNIPRO_HSI       6

When samsung_clk_register_gate() iterates over hsi_gate_clks and registers
IDs up to 6 (CLK_GOUT_UFS_EMBD_UNIPRO_HSI), it appears it will write out of
bounds:

drivers/clk/samsung/clk.c:samsung_clk_add_lookup() {
    ...
    ctx->clk_data.hws[id] = clk_hw;
    ...
}

Should CLKS_NR_HSI be defined using the highest clock ID instead to prevent
memory corruption when this node probes?

> +			compatible = "samsung,exynos8855-cmu-hsi";
> +			reg = <0x17000000 0x10000>;
> +			#clock-cells = <1>;
> +
> +			clocks = <&oscclk>,
> +				 <&cmu_top CLKCMU_DOUT_HSI_NOC>,
> +				 <&cmu_top CLKCMU_DOUT_HSI_UFS_EMBD>;
> +			clock-names = "oscclk",
> +				      "bus",
> +				      "ufs_embd";
> +			samsung,sysreg = <&sysreg_hsi>;

[Severity: Low]
Will this cause a device tree schema validation failure when running
make dtbs_check?

The samsung,sysreg property does not appear to be defined in the schema at
Documentation/devicetree/bindings/clock/samsung,exynos8855-clock.yaml,
which restricts additions globally via additionalProperties: false.

> +		};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009120137.2801911-1-raghav.s@samsung.com?part=13