[PATCH v2] mtd: diskonchip: Cast an operand to uint64_t to prevent potential uint32_t overflow in inftl_partscan()

[PATCH v2] mtd: diskonchip: Cast an operand to uint64_t to prevent potential uint32_t overflow in inftl_partscan()

[Gax-c]

From: Zichen Xie <zichenxie0106@gmail.com>

This was found by a static analyzer.
There may be a potential integer overflow issue in
inftl_partscan(). parts[0].size is defined as "uint64_t"
while mtd->erasesize and ip->firstUnit are defined as 32-bit
unsigned integer. The result of the calculation will be limited
to 32 bits without correct casting.
So, we recommend adding an extra cast to prevent potential
integer overflow.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
---
v2: correct "Fixes" tag.
---
 drivers/mtd/nand/raw/diskonchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/diskonchip.c b/drivers/mtd/nand/raw/diskonchip.c
index 8db7fc424571..70d6c2250f32 100644
--- a/drivers/mtd/nand/raw/diskonchip.c
+++ b/drivers/mtd/nand/raw/diskonchip.c
@@ -1098,7 +1098,7 @@ static inline int __init inftl_partscan(struct mtd_info *mtd, struct mtd_partiti
 		    (i == 0) && (ip->firstUnit > 0)) {
 			parts[0].name = " DiskOnChip IPL / Media Header partition";
 			parts[0].offset = 0;
-			parts[0].size = mtd->erasesize * ip->firstUnit;
+			parts[0].size = (uint64_t)mtd->erasesize * ip->firstUnit;
 			numparts = 1;
 		}
 
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: diskonchip: Cast an operand to uint64_t to prevent potential uint32_t overflow in inftl_partscan()

[Miquel Raynal]

Hi,

zichenxie0106@gmail.com wrote on Mon, 21 Oct 2024 14:27:54 -0500:

> From: Zichen Xie <zichenxie0106@gmail.com>
> 
> This was found by a static analyzer.

This should be the last sentence, not the first, maybe?

> There may be a potential integer overflow issue in
> inftl_partscan(). parts[0].size is defined as "uint64_t"
> while mtd->erasesize and ip->firstUnit are defined as 32-bit
> unsigned integer. The result of the calculation will be limited
> to 32 bits without correct casting.
> So, we recommend adding an extra cast to prevent potential
> integer overflow.

The wrapping is odd.

> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
> ---
> v2: correct "Fixes" tag.

If you want to provide a fix, you probably want to Cc: stable as well.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: diskonchip: Cast an operand to uint64_t to prevent potential uint32_t overflow in inftl_partscan()

[Zichen Xie]

On 2024/10/22 3:41, Miquel Raynal wrote:
> Hi,
>
> zichenxie0106@gmail.com wrote on Mon, 21 Oct 2024 14:27:54 -0500:
>
>> From: Zichen Xie <zichenxie0106@gmail.com>
>>
>> This was found by a static analyzer.
> This should be the last sentence, not the first, maybe?
I saw a lot patches making it as the first sentence. So, I did it too.
>
>> There may be a potential integer overflow issue in
>> inftl_partscan(). parts[0].size is defined as "uint64_t"
>> while mtd->erasesize and ip->firstUnit are defined as 32-bit
>> unsigned integer. The result of the calculation will be limited
>> to 32 bits without correct casting.
>> So, we recommend adding an extra cast to prevent potential
>> integer overflow.
> The wrapping is odd.
Sorry, but could you please point out where's odd so I can fix it.
>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
>> ---
>> v2: correct "Fixes" tag.
> If you want to provide a fix, you probably want to Cc: stable as well.

Yeah, I will add that to the next version.


Best,

Zichen

>
> Thanks,
> Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: diskonchip: Cast an operand to uint64_t to prevent potential uint32_t overflow in inftl_partscan()

[Miquel Raynal]

Hi Zichen,

zichenxie0106@gmail.com wrote on Tue, 22 Oct 2024 10:49:45 -0500:

> On 2024/10/22 3:41, Miquel Raynal wrote:
> > Hi,
> >
> > zichenxie0106@gmail.com wrote on Mon, 21 Oct 2024 14:27:54 -0500:
> >  
> >> From: Zichen Xie <zichenxie0106@gmail.com>
> >>
> >> This was found by a static analyzer.  
> > This should be the last sentence, not the first, maybe?  
> I saw a lot patches making it as the first sentence. So, I did it too.

Well, they are likely wrong.

Also, please shorten your commit title

"Cast an operand to prevent potential overflow" would be enough. You'll
give the details in the commit log anyway.

> >  
> >> There may be a potential integer overflow issue in
> >> inftl_partscan(). parts[0].size is defined as "uint64_t"
> >> while mtd->erasesize and ip->firstUnit are defined as 32-bit
> >> unsigned integer. The result of the calculation will be limited
> >> to 32 bits without correct casting.
> >> So, we recommend adding an extra cast to prevent potential
> >> integer overflow.  
> > The wrapping is odd.  
> Sorry, but could you please point out where's odd so I can fix it.

Your commit log is wrapped at 60-char.

> >  
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
> >> ---
> >> v2: correct "Fixes" tag.  
> > If you want to provide a fix, you probably want to Cc: stable as well.  
> 
> Yeah, I will add that to the next version.
> 
> 
> Best,
> 
> Zichen
> 
> >
> > Thanks,
> > Miquèl  


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/