From: Maxime Ripard maxime@cerno.tech
[ Upstream commit 10c46f2ea914202482d19cf80dcc9c321c9ff59b ]
If we were to have two users of the same clock, doing something like:
clk_set_rate_range(user1, 1000, 2000); clk_set_rate_range(user2, 3000, 4000);
The second call would fail with -EINVAL, preventing from getting in a situation where we end up with impossible limits.
However, this is never explicitly checked against and enforced, and works by relying on an undocumented behaviour of clk_set_rate().
Indeed, on the first clk_set_rate_range will make sure the current clock rate is within the new range, so it will be between 1000 and 2000Hz. On the second clk_set_rate_range(), it will consider (rightfully), that our current clock is outside of the 3000-4000Hz range, and will call clk_core_set_rate_nolock() to set it to 3000Hz.
clk_core_set_rate_nolock() will then call clk_calc_new_rates() that will eventually check that our rate 3000Hz rate is outside the min 3000Hz max 2000Hz range, will bail out, the error will propagate and we'll eventually return -EINVAL.
This solely relies on the fact that clk_calc_new_rates(), and in particular clk_core_determine_round_nolock(), won't modify the new rate allowing the error to be reported. That assumption won't be true for all drivers, and most importantly we'll break that assumption in a later patch.
It can also be argued that we shouldn't even reach the point where we're calling clk_core_set_rate_nolock().
Let's make an explicit check for disjoints range before we're doing anything.
Signed-off-by: Maxime Ripard maxime@cerno.tech Link: https://lore.kernel.org/r/20220225143534.405820-4-maxime@cerno.tech Signed-off-by: Stephen Boyd sboyd@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/clk/clk.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index ccb26a513b29..13332f89e034 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -637,6 +637,24 @@ static void clk_core_get_boundaries(struct clk_core *core, *max_rate = min(*max_rate, clk_user->max_rate); }
+static bool clk_core_check_boundaries(struct clk_core *core, + unsigned long min_rate, + unsigned long max_rate) +{ + struct clk *user; + + lockdep_assert_held(&prepare_lock); + + if (min_rate > core->max_rate || max_rate < core->min_rate) + return false; + + hlist_for_each_entry(user, &core->clks, clks_node) + if (min_rate > user->max_rate || max_rate < user->min_rate) + return false; + + return true; +} + void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate, unsigned long max_rate) { @@ -2306,6 +2324,11 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max) clk->min_rate = min; clk->max_rate = max;
+ if (!clk_core_check_boundaries(clk->core, min, max)) { + ret = -EINVAL; + goto out; + } + rate = clk_core_get_rate_nolock(clk->core); if (rate < min || rate > max) { /* @@ -2334,6 +2357,7 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max) } }
+out: if (clk->exclusive_count) clk_core_rate_protect(clk->core);