[PATCH] clk: qcom: clk-pll: reject vote enable on orphan parent

[PATCH] clk: qcom: clk-pll: reject vote enable on orphan parent

[Herman van Hazendonk]

clk_pll_vote_enable() unconditionally feeds the result of
clk_hw_get_parent(hw) through to_clk_pll() and on to wait_for_pll().
The common clock framework permits clk_enable() on an orphan clock
(supplier not bound yet), in which case clk_hw_get_parent() returns
NULL. to_clk_pll(NULL) then yields container_of(NULL, struct clk_pll,
clkr) -- a non-NULL bogus pointer pointing into the negative offset
of struct clk_pll.

wait_for_pll() reaches for the parent's name via
clk_hw_get_name(&pll->clkr.hw). Because clkr sits at a fixed offset
inside struct clk_pll, &pll->clkr.hw cancels the to_clk_pll offset
exactly back to NULL and clk_hw_get_name() then dereferences
core->name on a NULL clk_hw, panicking the kernel.

This is reachable today: gcc-msm8960.c and gcc-apq8064.c register a
pll4_vote whose parent (pll4) lives in lcc-msm8960.c, and the future
gcc-msm8660 pll4_vote does the same. If anything calls clk_enable()
on pll4_vote between gcc probe and the LCC clock controller binding,
the system panics. The exposure widens as more SoCs adopt the same
cross-controller voter pattern.

Resolve the parent with clk_hw_get_parent() once, return -ENODEV when
it is NULL, and only call into wait_for_pll() with a real
struct clk_pll. The enable-regmap write is also gated behind the
parent check so a failed enable cannot leave the vote bit asserted
against a clock the framework has not finished wiring up.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/clk/qcom/clk-pll.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-pll.c b/drivers/clk/qcom/clk-pll.c
index 26ba709f43c8..7b26129565fe 100644
--- a/drivers/clk/qcom/clk-pll.c
+++ b/drivers/clk/qcom/clk-pll.c
@@ -199,14 +199,31 @@ static int wait_for_pll(struct clk_pll *pll)
 
 static int clk_pll_vote_enable(struct clk_hw *hw)
 {
+	struct clk_hw *parent;
 	int ret;
-	struct clk_pll *p = to_clk_pll(clk_hw_get_parent(hw));
+
+	/*
+	 * Vote clocks can be registered on one clock controller and have
+	 * the underlying PLL live on a different one (e.g. PLL4_VOTE in
+	 * GCC for the LPASS PLL4 owned by LCC on the MSM8x60 / MSM8960 /
+	 * APQ8064 families). The common clock framework permits enable
+	 * on an orphan, so clk_hw_get_parent() can legitimately return
+	 * NULL here while the supplier controller has not finished
+	 * probing yet. Reject the enable rather than handing a bogus
+	 * container_of(NULL, struct clk_pll, clkr) pointer to
+	 * wait_for_pll() - inside wait_for_pll(), clk_hw_get_name()
+	 * would reverse the offset back to NULL and dereference
+	 * core->name.
+	 */
+	parent = clk_hw_get_parent(hw);
+	if (!parent)
+		return -ENODEV;
 
 	ret = clk_enable_regmap(hw);
 	if (ret)
 		return ret;
 
-	return wait_for_pll(p);
+	return wait_for_pll(to_clk_pll(parent));
 }
 
 const struct clk_ops clk_pll_vote_ops = {

base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
-- 
2.43.0