All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/sun4i: Fix an ulong overflow in the dotclock driver
@ 2018-10-18  9:25 ` Boris Brezillon
  0 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-10-18  9:25 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai; +Cc: dri-devel, Boris Brezillon, stable

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;
+
 		rounded = clk_hw_round_rate(clk_hw_get_parent(hw),
 					    ideal);
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] drm/sun4i: Fix an ulong overflow in the dotclock driver
@ 2018-10-18  9:25 ` Boris Brezillon
  0 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-10-18  9:25 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai; +Cc: Boris Brezillon, stable, dri-devel

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;
+
 		rounded = clk_hw_round_rate(clk_hw_get_parent(hw),
 					    ideal);
 
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/sun4i: Fix an ulong overflow in the dotclock driver
  2018-10-18  9:25 ` Boris Brezillon
@ 2018-10-18  9:49   ` Maxime Ripard
  -1 siblings, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2018-10-18  9:49 UTC (permalink / raw)
  To: Boris Brezillon; +Cc: Chen-Yu Tsai, dri-devel, stable

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] drm/sun4i: Fix an ulong overflow in the dotclock driver
@ 2018-10-18  9:49   ` Maxime Ripard
  0 siblings, 0 replies; 4+ messages in thread
From: Maxime Ripard @ 2018-10-18  9:49 UTC (permalink / raw)
  To: Boris Brezillon; +Cc: Chen-Yu Tsai, stable, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1610 bytes --]

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

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-18 17:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-18  9:25 [PATCH] drm/sun4i: Fix an ulong overflow in the dotclock driver Boris Brezillon
2018-10-18  9:25 ` Boris Brezillon
2018-10-18  9:49 ` Maxime Ripard
2018-10-18  9:49   ` Maxime Ripard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.