linux-kernel - [PATCH] clk: sunxi: Fix mod0 clock calculation to return stable results and check divisor size limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 28 Dec 2015 18:31:32 +0100
From:	Marcus Weseloh <mweseloh42@...il.com>
To:	linux-sunxi@...glegroups.com
Cc:	Marcus Weseloh <mweseloh42@...il.com>,
	Emilio López <emilio@...pez.com.ar>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Chen-Yu Tsai <wens@...e.org>, linux-clk@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH] clk: sunxi: Fix mod0 clock calculation to return stable results and check divisor size limits

This patch fixes some problems in the mod0 clock calculation. It has
the potential to break stuff, as the issues explained below had the
effect that clk_set_rate would always return successfully, sometimes
setting a frequency that is higher than the requested value. Code that
"accidentally worked" because of this might fail after applying
this patch.

The problems in detail:

1. If a very low frequency is requested from a high parent clock, the
divisors "div" and "calcm" might be > 255. This patch changes the type
of both variables to unsigned int, because the silent cast to u8 will
result in invalid frequencies and register values.

2. The width of the "m" divisor in the clock control registers is only
4 bit, but that limitation is not checked when calculating the divisor
and the resulting frequency. This patch adds a check that m never
exceeds the field width.

3. During a call to clk_set_rate, the sun4i_a10_get_mod0_factors
function is called multiple times: first to find the best parent and
frequency, then again to calculate the p and m divisors, passing the
frequencies returned by the previous call(s). In certain cases
those chained calls do not result in the best frequency choice.

An example:
parent_rate = 24Mhz, freq = 1.4Mhz results in p=1, m=9, freq=1333333,3333
(which gets rounded down to 1333333).
Calling the function again with parent_rate = 24Mhz and freq = 1333333
results in p=1, m=10, freq=1200000.

Rounding up the returned frequency removes this problem.

Signed-off-by: Marcus Weseloh <mweseloh42@...il.com>
---
 drivers/clk/sunxi/clk-mod0.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi/clk-mod0.c b/drivers/clk/sunxi/clk-mod0.c
index d167e1e..d03f099 100644
--- a/drivers/clk/sunxi/clk-mod0.c
+++ b/drivers/clk/sunxi/clk-mod0.c
@@ -31,7 +31,8 @@
 static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 				       u8 *n, u8 *k, u8 *m, u8 *p)
 {
-	u8 div, calcm, calcp;
+	unsigned int div, calcm;
+	u8 calcp;

 	/* These clocks can only divide, so we will never be able to achieve
 	 * frequencies higher than the parent frequency */
@@ -50,8 +51,10 @@ static void sun4i_a10_get_mod0_factors(u32 *freq, u32 parent_rate,
 		calcp = 3;

 	calcm = DIV_ROUND_UP(div, 1 << calcp);
+	if (calcm > 16)
+		calcm = 16;

-	*freq = (parent_rate >> calcp) / calcm;
+	*freq = DIV_ROUND_UP(parent_rate >> calcp, calcm);

 	/* we were called to round the frequency, we can now return */
 	if (n == NULL)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/