diff mbox

mmc: sdhci: cast unsigned int to unsigned long long to avoid unexpeted error

Message ID 1473750418-23111-1-git-send-email-haibo.chen@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bough Chen Sept. 13, 2016, 7:06 a.m. UTC
Potentially overflowing expression 1000000 * data->timeout_clks with
type unsigned int is evaluated using 32-bit arithmetic, and then used
in a context that expects an expression of type unsigned long long.

To avoid overflow, cast 1000000U to type unsigned long long.
Special thanks to coverity <http://www.coverity.com>

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Adrian Hunter Oct. 14, 2016, 9:12 a.m. UTC | #1
On 13/09/16 11:36, Adrian Hunter wrote:
> On 13/09/16 10:06, Haibo Chen wrote:
>> Potentially overflowing expression 1000000 * data->timeout_clks with
>> type unsigned int is evaluated using 32-bit arithmetic, and then used
>> in a context that expects an expression of type unsigned long long.
>>
>> To avoid overflow, cast 1000000U to type unsigned long long.
>> Special thanks to coverity <http://www.coverity.com>
> 
> Let's leave out the web link - it is enough to say it was found with Coverity.
> 
>>
>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Do you plan to re-submit this?

> 
> Needs a fixes tag and cc stable.
> 
>> ---
>>  drivers/mmc/host/sdhci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 4805566..0e9f495 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -687,7 +687,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  			 * host->clock is in Hz.  target_timeout is in us.
>>  			 * Hence, us = 1000000 * cycles / Hz.  Round up.
>>  			 */
>> -			val = 1000000 * data->timeout_clks;
>> +			val = (unsigned long long)1000000 * data->timeout_clks;
> 
> Please use 1000000ULL instead of a cast.
> 
>>  			if (do_div(val, host->clock))
>>  				target_timeout++;
>>  			target_timeout += val;
>>
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bough Chen Oct. 14, 2016, 9:53 a.m. UTC | #2
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Adrian Hunter
> Sent: Friday, October 14, 2016 5:12 PM
> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org
> Cc: linux-mmc@vger.kernel.org
> Subject: Re: [PATCH] mmc: sdhci: cast unsigned int to unsigned long long to
> avoid unexpeted error
> 
> On 13/09/16 11:36, Adrian Hunter wrote:
> > On 13/09/16 10:06, Haibo Chen wrote:
> >> Potentially overflowing expression 1000000 * data->timeout_clks with
> >> type unsigned int is evaluated using 32-bit arithmetic, and then used
> >> in a context that expects an expression of type unsigned long long.
> >>
> >> To avoid overflow, cast 1000000U to type unsigned long long.
> >> Special thanks to coverity <http://www.coverity.com>
> >
> > Let's leave out the web link - it is enough to say it was found with Coverity.
> >
> >>
> >> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> 
> Do you plan to re-submit this?
> 

Yes, I will re-submit this soon. 

> >
> > Needs a fixes tag and cc stable.
> >
> >> ---
> >>  drivers/mmc/host/sdhci.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> index 4805566..0e9f495 100644
> >> --- a/drivers/mmc/host/sdhci.c
> >> +++ b/drivers/mmc/host/sdhci.c
> >> @@ -687,7 +687,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host,
> struct mmc_command *cmd)
> >>  			 * host->clock is in Hz.  target_timeout is in us.
> >>  			 * Hence, us = 1000000 * cycles / Hz.  Round up.
> >>  			 */
> >> -			val = 1000000 * data->timeout_clks;
> >> +			val = (unsigned long long)1000000 * data->timeout_clks;
> >
> > Please use 1000000ULL instead of a cast.
> >
> >>  			if (do_div(val, host->clock))
> >>  				target_timeout++;
> >>  			target_timeout += val;
> >>
> >
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4805566..0e9f495 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -687,7 +687,7 @@  static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 			 * host->clock is in Hz.  target_timeout is in us.
 			 * Hence, us = 1000000 * cycles / Hz.  Round up.
 			 */
-			val = 1000000 * data->timeout_clks;
+			val = (unsigned long long)1000000 * data->timeout_clks;
 			if (do_div(val, host->clock))
 				target_timeout++;
 			target_timeout += val;