Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clk: at91: out-of-bounds write and lookup failure in sama7g5_pmc_setup()
- [High] clk: at91: uninitialized hw pointer stored in sama7g5_pmc_setup()
--
commit 449771270ea59bc4c7759cd22fba4a55741f936a
Author: Alexander Dahl [off-list ref]
ARM: dts: microchip: sama7g5: Add OTPC clocks
This patch adds the main RC oscillator and OTPC peripheral clocks to the
otpc node in the SAMA7G5 device tree.
Link: https://lore.kernel.org/linux-clk/ec34efc2-2051-4b8a-b5d8-6e2fd5e08c28@microchip.com/T/#u (local)
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/boot/dts/microchip/sama7g5.dtsi b/arch/arm/boot/dts/microchip/sama7g5.dtsi
index b8296391fc696..30193f3bf9775 100644
--- a/arch/arm/boot/dts/microchip/sama7g5.dtsi
+++ b/arch/arm/boot/dts/microchip/sama7g5.dtsi
@@ -1036,6 +1036,7 @@ otpc: efuse@e8c00000 {
reg = <0xe8c00000 0x100>;
#address-cells = <1>;
#size-cells = <1>;
+ clocks = <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>, <&pmc PMC_TYPE_PERIPHERAL 67>;
[Severity: High]
Can mchp_otpc_probe() acquire SAMA7G5_PMC_MAIN_RC here, or does this cause
the OTPC probe to fail with -EINVAL due to an out-of-bounds core clock
index in sama7g5_pmc_setup()?
In include/dt-bindings/clock/microchip,sama7g5-pmc.h, SAMA7G5_PMC_MCK1 is
13 and SAMA7G5_PMC_MAIN_RC is 14. However, in
drivers/clk/at91/sama7g5.c:sama7g5_pmc_setup(), sama7g5_pmc is allocated
with ncore set to SAMA7G5_PMC_MCK1 + 1 (14):
sama7g5_pmc = pmc_data_allocate(SAMA7G5_PMC_MCK1 + 1,
nck(sama7g5_systemck),
nck(sama7g5_periphck),
nck(sama7g5_gck), 8);
Because chws only has 14 entries (indices 0..13), writing to index
SAMA7G5_PMC_MAIN_RC (14) in sama7g5_pmc_setup() writes one element past the
end of sama7g5_pmc->chws[] into sama7g5_pmc->shws[0]:
sama7g5_pmc->chws[SAMA7G5_PMC_MAIN_RC] = hw;
Then, when mchp_otpc_probe() calls devm_clk_bulk_get_all_enabled(),
drivers/clk/at91/pmc.c:of_clk_hw_pmc_get() checks:
case PMC_TYPE_CORE:
if (idx < pmc_data->ncore)
return pmc_data->chws[idx];
break;
Since idx is 14 and pmc_data->ncore is 14, of_clk_hw_pmc_get() returns
ERR_PTR(-EINVAL), failing the OTPC probe on SAMA7G5.
[Severity: High]
Also in drivers/clk/at91/sama7g5.c:sama7g5_pmc_setup() (and similarly in
drivers/clk/at91/sama7d65.c:sama7d65_pmc_setup()), when registering the
main RC oscillator:
main_rc_hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000,
50000000);
if (IS_ERR(main_rc_hw))
goto err_free;
sama7g5_pmc->chws[SAMA7G5_PMC_MAIN_RC] = hw;
Doesn't this store the uninitialized local pointer hw instead of main_rc_hw
into chws[SAMA7G5_PMC_MAIN_RC], returning a garbage clk_hw pointer when the
OTPC node requests <&pmc PMC_TYPE_CORE SAMA7G5_PMC_MAIN_RC>?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261105110715.2486432-1-ada@thorsis.com?part=13