Re: [PATCH 2/3] clk: samsung: clk-pll: Add support for pll_309
From: sashiko-bot@kernel.org
Date: 2026-08-17 10:37:12
Also in:
linux-clk
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Logic Error / Out-of-bounds Read when rate_table is empty - [Medium] Single-Responsibility Violation: Undocumented architectural refactoring --
--- Patch [2]: [PATCH 2/3] clk: samsung: clk-pll: Add support for pll_309 ---commit d62df56ff2e6bf73437e6e25e1335b3fb4571268 Author: Aiden Isik [off-list ref] clk: samsung: clk-pll: Add support for pll_309 This commit adds support for the pll_309 used in the Exynos5515 SoC, defining the PLL characteristics which are handled similarly to pll_531x.
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c index e74552846ba34..8b1474aeaf0c8 100644 --- a/drivers/clk/samsung/clk-pll.c +++ b/drivers/clk/samsung/clk-pll.c@@ -1593,32 +1593,35 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx, { struct samsung_clk_pll *pll; struct clk_init_data init; - unsigned int len = 0; - int ret; + int ret, len; - if (pll_clk->rate_table) { - /* find count of rates in rate_table */ - while (pll_clk->rate_table[len].rate != 0) - len++; - } - - pll = kzalloc_flex(*pll, rate_table, len); + pll = kzalloc_obj(*pll);
[Severity: Medium] This isn't a bug, but the commit message only mentions adding support for the new pll_309 type, while this code changes the allocation semantics of the rate_table across the entire driver, switching from a flexible array member to a separate pointer allocation. Could this architectural refactoring be split into a separate patch or documented in the commit message? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1637,7 +1640,7 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx, case pll_a9fracm: pll->enable_offs = PLL35XX_ENABLE_SHIFT; pll->lock_offs = PLL35XX_LOCK_STAT_SHIFT; - if (!pll->rate_count) + if (!pll->rate_table)
[Severity: High]
If a clock is registered with an empty rate table containing only the zero
terminator, kmemdup_array() is called with a count of 0 earlier in this
function and returns ZERO_SIZE_PTR rather than NULL.
Since ZERO_SIZE_PTR is not NULL, this check evaluates to false, which
incorrectly assigns the full operations instead of the safe min_ops fallback.
If samsung_pll_determine_rate() is subsequently called, it skips its loop
because rate_count is 0, and attempts to access rate_table[-1]:
samsung_pll_determine_rate() {
...
req->rate = rate_table[i - 1].rate;
}
Since rate_table is ZERO_SIZE_PTR, will this result in an out-of-bounds
memory read?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-for-next-lucky7-clock-v1-0-032e4b229c76@member.fsf.org?part=2