* [ScsiDiskDxe] Implict type conversion bug
@ 2017-04-10 9:40 Guoheyi
2017-04-10 9:48 ` Zeng, Star
0 siblings, 1 reply; 5+ messages in thread
From: Guoheyi @ 2017-04-10 9:40 UTC (permalink / raw)
To: edk2-devel@lists.01.org; +Cc: Feng Tian, Feng Tian
Hi folks,
In MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c, Line 2757, we have the code as below:
ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) |
(Capacity10->LastLba2 << 16) |
(Capacity10->LastLba1 << 8) |
Capacity10->LastLba0;
However, we tested on VS2008 and found that the intermediate result would be convert to INT32 then INT64 implicitly, so the result will be sign extended and LastBlock may not be assigned with what is expected. We can simply test LastLba3 >= 0x80 to see what will happen.
Please let me know your comments.
Thanks and regards,
Gary (Heyi Guo)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [ScsiDiskDxe] Implict type conversion bug 2017-04-10 9:40 [ScsiDiskDxe] Implict type conversion bug Guoheyi @ 2017-04-10 9:48 ` Zeng, Star 2017-04-11 4:24 ` 答复: " Guoheyi 2017-04-11 7:05 ` Guoheyi 0 siblings, 2 replies; 5+ messages in thread From: Zeng, Star @ 2017-04-10 9:48 UTC (permalink / raw) To: Guoheyi, edk2-devel@lists.01.org; +Cc: Tian, Feng, Zeng, Star Hao has sent patch for this issue. You can search emails with title "[edk2] [PATCH] MdeModulePkg/ScsiDiskDxe: Fix potential implicit sign extension". Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Guoheyi Sent: Monday, April 10, 2017 5:40 PM To: edk2-devel@lists.01.org Cc: Tian, Feng <feng.tian@intel.com> Subject: [edk2] [ScsiDiskDxe] Implict type conversion bug Hi folks, In MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c, Line 2757, we have the code as below: ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) | (Capacity10->LastLba2 << 16) | (Capacity10->LastLba1 << 8) | Capacity10->LastLba0; However, we tested on VS2008 and found that the intermediate result would be convert to INT32 then INT64 implicitly, so the result will be sign extended and LastBlock may not be assigned with what is expected. We can simply test LastLba3 >= 0x80 to see what will happen. Please let me know your comments. Thanks and regards, Gary (Heyi Guo) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* 答复: [ScsiDiskDxe] Implict type conversion bug 2017-04-10 9:48 ` Zeng, Star @ 2017-04-11 4:24 ` Guoheyi 2017-04-11 7:05 ` Guoheyi 1 sibling, 0 replies; 5+ messages in thread From: Guoheyi @ 2017-04-11 4:24 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Tian, Feng Thanks for your quick fix :) -----邮件原件----- 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2017年4月10日 17:48 收件人: Guoheyi; edk2-devel@lists.01.org 抄送: Tian, Feng; Zeng, Star 主题: RE: [edk2] [ScsiDiskDxe] Implict type conversion bug Hao has sent patch for this issue. You can search emails with title "[edk2] [PATCH] MdeModulePkg/ScsiDiskDxe: Fix potential implicit sign extension". Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Guoheyi Sent: Monday, April 10, 2017 5:40 PM To: edk2-devel@lists.01.org Cc: Tian, Feng <feng.tian@intel.com> Subject: [edk2] [ScsiDiskDxe] Implict type conversion bug Hi folks, In MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c, Line 2757, we have the code as below: ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) | (Capacity10->LastLba2 << 16) | (Capacity10->LastLba1 << 8) | Capacity10->LastLba0; However, we tested on VS2008 and found that the intermediate result would be convert to INT32 then INT64 implicitly, so the result will be sign extended and LastBlock may not be assigned with what is expected. We can simply test LastLba3 >= 0x80 to see what will happen. Please let me know your comments. Thanks and regards, Gary (Heyi Guo) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* 答复: [ScsiDiskDxe] Implict type conversion bug 2017-04-10 9:48 ` Zeng, Star 2017-04-11 4:24 ` 答复: " Guoheyi @ 2017-04-11 7:05 ` Guoheyi 2017-04-11 7:13 ` Wu, Hao A 1 sibling, 1 reply; 5+ messages in thread From: Guoheyi @ 2017-04-11 7:05 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Tian, Feng Hi Star, I searched " LastLba3" in EDK2 code and found there were some other code similar to this one, could you please check whether they need to be fixed as well? IntelFrameworkModulePkg/Bus/Pci/IdeBusDxe/Atapi.c: IdeDev->BlkIo.Media->LastBlock = (Data.LastLba3 << 24) | IntelFrameworkModulePkg/Bus/Pci/IdeBusDxe/Atapi.c: IdeDev->BlkIo.Media->LastBlock = (FormatData.LastLba3 << 24) | MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c: Private->Media[DeviceIndex].LastBlock = (Capacity16.LastLba3 << 24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | Capacity16.LastLba0; MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c: Private->Media[DeviceIndex].LastBlock = (Capacity.LastLba3 << 24) | (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0; MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c: MediaInfo->LastBlock = (Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0; MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c: MediaInfo->LastBlock = (FormatData.LastLba3 << 24) | (I think the two lines below should be OK for LastBlock is defined as UINT32) MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c: LastBlock = (Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0; MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c: LastBlock = (FormatData.LastLba3 << 24) | (FormatData.LastLba2 << 16) | (FormatData.LastLba1 << 8) | FormatData.LastLba0; Thanks and regards, Gary (Heyi Guo) -----邮件原件----- 发件人: Guoheyi 发送时间: 2017年4月11日 12:25 收件人: 'Zeng, Star'; edk2-devel@lists.01.org 抄送: Tian, Feng 主题: 答复: [edk2] [ScsiDiskDxe] Implict type conversion bug Thanks for your quick fix :) -----邮件原件----- 发件人: Zeng, Star [mailto:star.zeng@intel.com] 发送时间: 2017年4月10日 17:48 收件人: Guoheyi; edk2-devel@lists.01.org 抄送: Tian, Feng; Zeng, Star 主题: RE: [edk2] [ScsiDiskDxe] Implict type conversion bug Hao has sent patch for this issue. You can search emails with title "[edk2] [PATCH] MdeModulePkg/ScsiDiskDxe: Fix potential implicit sign extension". Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Guoheyi Sent: Monday, April 10, 2017 5:40 PM To: edk2-devel@lists.01.org Cc: Tian, Feng <feng.tian@intel.com> Subject: [edk2] [ScsiDiskDxe] Implict type conversion bug Hi folks, In MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c, Line 2757, we have the code as below: ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) | (Capacity10->LastLba2 << 16) | (Capacity10->LastLba1 << 8) | Capacity10->LastLba0; However, we tested on VS2008 and found that the intermediate result would be convert to INT32 then INT64 implicitly, so the result will be sign extended and LastBlock may not be assigned with what is expected. We can simply test LastLba3 >= 0x80 to see what will happen. Please let me know your comments. Thanks and regards, Gary (Heyi Guo) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 答复: [ScsiDiskDxe] Implict type conversion bug 2017-04-11 7:05 ` Guoheyi @ 2017-04-11 7:13 ` Wu, Hao A 0 siblings, 0 replies; 5+ messages in thread From: Wu, Hao A @ 2017-04-11 7:13 UTC (permalink / raw) To: Guoheyi, Zeng, Star, edk2-devel@lists.01.org; +Cc: Tian, Feng Hi Gary, Thanks for the reporting, I will take a look at all the cases you mentioned. Best Regards, Hao Wu > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Guoheyi > Sent: Tuesday, April 11, 2017 3:05 PM > To: Zeng, Star; edk2-devel@lists.01.org > Cc: Tian, Feng > Subject: [edk2] 答复: [ScsiDiskDxe] Implict type conversion bug > > Hi Star, > > I searched " LastLba3" in EDK2 code and found there were some other code > similar to this one, could you please check whether they need to be fixed as > well? > > IntelFrameworkModulePkg/Bus/Pci/IdeBusDxe/Atapi.c: IdeDev- > >BlkIo.Media->LastBlock = (Data.LastLba3 << 24) | > IntelFrameworkModulePkg/Bus/Pci/IdeBusDxe/Atapi.c: IdeDev- > >BlkIo.Media->LastBlock = (FormatData.LastLba3 << 24) | > MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c: Private- > >Media[DeviceIndex].LastBlock = (Capacity16.LastLba3 << 24) | > (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | > Capacity16.LastLba0; > MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c: Private- > >Media[DeviceIndex].LastBlock = (Capacity.LastLba3 << 24) | > (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0; > MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c: MediaInfo->LastBlock = > (Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | > Data.LastLba0; > MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c: MediaInfo->LastBlock = > (FormatData.LastLba3 << 24) | > > (I think the two lines below should be OK for LastBlock is defined as UINT32) > MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c: LastBlock = (Data.LastLba3 << > 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0; > MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c: LastBlock = > (FormatData.LastLba3 << 24) | (FormatData.LastLba2 << 16) | > (FormatData.LastLba1 << 8) | FormatData.LastLba0; > > > Thanks and regards, > > Gary (Heyi Guo) > > > -----邮件原件----- > 发件人: Guoheyi > 发送时间: 2017年4月11日 12:25 > 收件人: 'Zeng, Star'; edk2-devel@lists.01.org > 抄送: Tian, Feng > 主题: 答复: [edk2] [ScsiDiskDxe] Implict type conversion bug > > Thanks for your quick fix :) > > -----邮件原件----- > 发件人: Zeng, Star [mailto:star.zeng@intel.com] > 发送时间: 2017年4月10日 17:48 > 收件人: Guoheyi; edk2-devel@lists.01.org > 抄送: Tian, Feng; Zeng, Star > 主题: RE: [edk2] [ScsiDiskDxe] Implict type conversion bug > > Hao has sent patch for this issue. > You can search emails with title "[edk2] [PATCH] MdeModulePkg/ScsiDiskDxe: > Fix potential implicit sign extension". > > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Guoheyi > Sent: Monday, April 10, 2017 5:40 PM > To: edk2-devel@lists.01.org > Cc: Tian, Feng <feng.tian@intel.com> > Subject: [edk2] [ScsiDiskDxe] Implict type conversion bug > > Hi folks, > > In MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c, Line 2757, we have the > code as below: > > ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) | > (Capacity10->LastLba2 << 16) | > (Capacity10->LastLba1 << 8) | > Capacity10->LastLba0; > > However, we tested on VS2008 and found that the intermediate result would be > convert to INT32 then INT64 implicitly, so the result will be sign extended and > LastBlock may not be assigned with what is expected. We can simply test > LastLba3 >= 0x80 to see what will happen. > > Please let me know your comments. > > Thanks and regards, > > Gary (Heyi Guo) > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-11 7:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-10 9:40 [ScsiDiskDxe] Implict type conversion bug Guoheyi 2017-04-10 9:48 ` Zeng, Star 2017-04-11 4:24 ` 答复: " Guoheyi 2017-04-11 7:05 ` Guoheyi 2017-04-11 7:13 ` Wu, Hao A
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox