[PATCH v2] drm/sun4i: Fix an ulong overflow in the dotclock driver

[PATCH v2] drm/sun4i: Fix an ulong overflow in the dotclock driver

[Boris Brezillon]

The calculated ideal rate can easily overflow an unsigned long, thus
making the best div selection buggy as soon as no ideal match is found
before the overflow occurs.

Fixes: 4731a72df273 ("drm/sun4i: request exact rates to our parents")
Cc: <stable@vger.kernel.org>
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
Changes in v2:
- Add a comment to explain why we bail out after an overflow
- Add Maxime ack
- Use a goto instead of a break
---
 drivers/gpu/drm/sun4i/sun4i_dotclock.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_dotclock.c b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
index e36004fbe453..2a15f2f9271e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_dotclock.c
+++ b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
@@ -81,9 +81,19 @@ static long sun4i_dclk_round_rate(struct clk_hw *hw, unsigned long rate,
 	int i;
 
 	for (i = tcon->dclk_min_div; i <= tcon->dclk_max_div; i++) {
-		unsigned long ideal = rate * i;
+		u64 ideal = (u64)rate * i;
 		unsigned long rounded;
 
+		/*
+		 * ideal has overflowed the max value that can be stored in an
+		 * unsigned long, and every clk operation we might do on a
+		 * truncated u64 value will give us incorrect results.
+		 * Let's just stop there since bigger dividers will result in
+		 * the same overflow issue.
+		 */
+		if (ideal > ULONG_MAX)
+			goto out;
+
 		rounded = clk_hw_round_rate(clk_hw_get_parent(hw),
 					    ideal);
 
-- 
2.14.1

Re: [PATCH v2] drm/sun4i: Fix an ulong overflow in the dotclock driver

[Maxime Ripard]

On Thu, Oct 18, 2018 at 12:02:50PM +0200, Boris Brezillon wrote:
> The calculated ideal rate can easily overflow an unsigned long, thus
> making the best div selection buggy as soon as no ideal match is found
> before the overflow occurs.
> 
> Fixes: 4731a72df273 ("drm/sun4i: request exact rates to our parents")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
> Changes in v2:
> - Add a comment to explain why we bail out after an overflow
> - Add Maxime ack
> - Use a goto instead of a break

APplied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com