diff mbox series

drm/sun4i: Fix an ulong overflow in the dotclock driver

Message ID 20181018092546.23078-1-boris.brezillon@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/sun4i: Fix an ulong overflow in the dotclock driver | expand

Commit Message

Boris Brezillon Oct. 18, 2018, 9:25 a.m. UTC
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>
---
 drivers/gpu/drm/sun4i/sun4i_dotclock.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Maxime Ripard Oct. 18, 2018, 9:49 a.m. UTC | #1
Hi!

On Thu, Oct 18, 2018 at 11:25:46AM +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>
> ---
>  drivers/gpu/drm/sun4i/sun4i_dotclock.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_dotclock.c b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
> index e36004fbe453..82132a9bd1d5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_dotclock.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
> @@ -81,9 +81,12 @@ 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;
>  
> +		if (ideal > ULONG_MAX)
> +			break;
> +

So I guess the rationale is that since we've overflown already, every
divider above that limit will make us overflow more and therefore
there's no point in going forward?

I'd be fine with that approach, but I'd like a comment explaining why
you have that test, and a goto out instead of the break for consistency.

with that fixed:
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Thanks!
Maxime
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_dotclock.c b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
index e36004fbe453..82132a9bd1d5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_dotclock.c
+++ b/drivers/gpu/drm/sun4i/sun4i_dotclock.c
@@ -81,9 +81,12 @@  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;
 
+		if (ideal > ULONG_MAX)
+			break;
+
 		rounded = clk_hw_round_rate(clk_hw_get_parent(hw),
 					    ideal);