[U-Boot] [PATCH] mmc: cat u8 to u64 to avoid unexpected error
Tom Rini
trini at konsulko.com
Mon Sep 19 13:30:20 CEST 2016
On Mon, Sep 19, 2016 at 03:31:54PM +0900, Jaehoon Chung wrote:
> On 09/19/2016 02:53 AM, Tom Rini wrote:
> > On Tue, Sep 13, 2016 at 04:27:57PM +0800, Haibo Chen wrote:
> >
> >> Suspicious implicit sign extension exist. ext_csd[] is defined
> >> as "u8", capacity is defined as u64, so u8 is promoted to signed
> >> int first int the "|" expersion, then the sign extended to u64.
> >> if the tmp sign value is largeer than 0x7fffffff, after the sign
> >> extension, the upper bits of the result will all be 1.
> >> Thanks to coverity <http://www.coverity.com>
> >>
> >> e.g.
> >> u8 data_8;
> >> u64 data_64;
> >>
> >> data_8 = 0x80;
> >> data_64 = data_8 << 24; //0xffffffff80000000
> >> data_64 = ((u64)data_8) << 24; //0x80000000
> >>
> >> Signed-off-by: Haibo Chen <haibo.chen at nxp.com>
> >
> > Please add a 'Reported-by: Coverity' and you can include the CID if you
> > like.
>
> I think cid doesn't need to change type.
I mean the coverity CID :) In the public coverity project it's 149300
>
> >
> >> ---
> >> drivers/mmc/mmc.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> >> index 43ea0bb..c1d1dc6 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -1176,10 +1176,10 @@ static int mmc_startup(struct mmc *mmc)
> >> * ext_csd's capacity is valid if the value is more
> >> * than 2GB
> >> */
> >> - capacity = ext_csd[EXT_CSD_SEC_CNT] << 0
> >> - | ext_csd[EXT_CSD_SEC_CNT + 1] << 8
> >> - | ext_csd[EXT_CSD_SEC_CNT + 2] << 16
> >> - | ext_csd[EXT_CSD_SEC_CNT + 3] << 24;
> >> + capacity = ((u64)ext_csd[EXT_CSD_SEC_CNT]) << 0
> >> + | ((u64)ext_csd[EXT_CSD_SEC_CNT + 1]) << 8
> >> + | ((u64)ext_csd[EXT_CSD_SEC_CNT + 2]) << 16
> >> + | ((u64)ext_csd[EXT_CSD_SEC_CNT + 3]) << 24;
> >> capacity *= MMC_MAX_BLOCK_LEN;
> >> if ((capacity >> 20) > 2 * 1024)
> >> mmc->capacity_user = capacity;
> >
> > Can't we just move capacity down to a u8 instead? Thanks!
>
> Maybe not to move down to a u8..because it's displayed the real capacity for storage.
We could update those lines too? It's just that one case right there,
yes?
> And i wonder that coverity didn't report about the line 1294?
It does, along with 1256.
Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160919/e7f07b1f/attachment.sig>
More information about the U-Boot
mailing list