On 09/19/2016 08:30 PM, Tom Rini wrote:
> 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.c...@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

Ah! I misunderstood CID as cid register. :)

> 
>>
>>>
>>>> ---
>>>>  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?

If it's possible to calculate the correct capacity?

Best Regards,
Jaehoon Chung

> 
>> And i wonder that coverity didn't report about the line 1294?
> 
> It does, along with 1256.
> 
> Thanks!
> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to