diff mbox

ASoC: fsl_spdif: Fix integer overflow when calculating divisors

Message ID 1402330603-16547-1-git-send-email-anssi.hannula@iki.fi (mailing list archive)
State Accepted
Commit c89c7e94bb7d89b39471c79034e3ba1b25d817f5
Headers show

Commit Message

Anssi Hannula June 9, 2014, 4:16 p.m. UTC
The calculation code does
u64 = (u32 - u32) * 100000;

The 64 bits are of no help here as the type is casted only after the
multiplication, and therefore the result may overflow, possibly causing
inoptimal or wrong clock setup in an unfortunate case (the maximum
result value of the first substraction is currently 47999).

Fix the code to cast before multiplication.

Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
Cc: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 sound/soc/fsl/fsl_spdif.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Nicolin Chen June 9, 2014, 5:26 p.m. UTC | #1
On Mon, Jun 09, 2014 at 07:16:43PM +0300, Anssi Hannula wrote:
> The calculation code does
> u64 = (u32 - u32) * 100000;
> 
> The 64 bits are of no help here as the type is casted only after the
> multiplication, and therefore the result may overflow, possibly causing
> inoptimal or wrong clock setup in an unfortunate case (the maximum
> result value of the first substraction is currently 47999).
> 
> Fix the code to cast before multiplication.
> 
> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> Cc: Nicolin Chen <Guangyu.Chen@freescale.com>

Acked-by: Nicolin Chen <Guangyu.Chen@freescale.com>

Thank you so much :)
Nicolin

> ---
>  sound/soc/fsl/fsl_spdif.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> index b912d45a2a4c..0bd9136279a2 100644
> --- a/sound/soc/fsl/fsl_spdif.c
> +++ b/sound/soc/fsl/fsl_spdif.c
> @@ -1076,7 +1076,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
>  				goto out;
>  			} else if (arate / rate[index] == 1) {
>  				/* A little bigger than expect */
> -				sub = (arate - rate[index]) * 100000;
> +				sub = (u64)(arate - rate[index]) * 100000;
>  				do_div(sub, rate[index]);
>  				if (sub >= savesub)
>  					continue;
> @@ -1086,7 +1086,7 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
>  				spdif_priv->txrate[index] = arate;
>  			} else if (rate[index] / arate == 1) {
>  				/* A little smaller than expect */
> -				sub = (rate[index] - arate) * 100000;
> +				sub = (u64)(rate[index] - arate) * 100000;
>  				do_div(sub, rate[index]);
>  				if (sub >= savesub)
>  					continue;
> -- 
> 1.8.4.5
>
Mark Brown June 9, 2014, 8 p.m. UTC | #2
On Mon, Jun 09, 2014 at 07:16:43PM +0300, Anssi Hannula wrote:
> The calculation code does
> u64 = (u32 - u32) * 100000;

Applied, thanks.
diff mbox

Patch

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index b912d45a2a4c..0bd9136279a2 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1076,7 +1076,7 @@  static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
 				goto out;
 			} else if (arate / rate[index] == 1) {
 				/* A little bigger than expect */
-				sub = (arate - rate[index]) * 100000;
+				sub = (u64)(arate - rate[index]) * 100000;
 				do_div(sub, rate[index]);
 				if (sub >= savesub)
 					continue;
@@ -1086,7 +1086,7 @@  static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
 				spdif_priv->txrate[index] = arate;
 			} else if (rate[index] / arate == 1) {
 				/* A little smaller than expect */
-				sub = (rate[index] - arate) * 100000;
+				sub = (u64)(rate[index] - arate) * 100000;
 				do_div(sub, rate[index]);
 				if (sub >= savesub)
 					continue;