Skip to content

Commit

Permalink
clk: Enforce that disjoints limits are invalid
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Maxime Ripard authored and Stephen Boyd committed Mar 12, 2022
1 parent 723d053 commit 10c46f2
Showing 1 changed file with 24 additions and 0 deletions.
24 changes: 24 additions & 0 deletions drivers/clk/clk.c
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,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)
{
Expand Down Expand Up @@ -2348,6 +2366,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) {
/*
Expand Down Expand Up @@ -2376,6 +2399,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);

Expand Down

0 comments on commit 10c46f2

Please sign in to comment.