tyler.durden
/
audk
mirror of https://github.com/acidanthera/audk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MdeModulePkg/IdeBusPei: Fix undefined behavior in signed left shift 

In function ReadCapacity(), the following expression:
MediaInfo->LastBlock = (Data.LastLba3 << 24) |
  (Data.LastLba2 << 16) |
  (Data.LastLba1 << 8) |
  Data.LastLba0;

(There is also a similar case in this function.)

will involve undefined behavior in signed left shift operations.

Since Data.LastLbaX is of type UINT8, and MediaInfo->LastBlock is of type
UINTN. Therefore, Data.LastLbaX will be promoted to int (32 bits, signed)
first, and then perform the left shift operation.

According to the C11 spec, Section 6.5.7:
4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
  bits are filled with zeros. If E1 has an unsigned type, the value
  of the result is E1 * 2^E2 , reduced modulo one more than the
  maximum value representable in the result type. If E1 has a signed
  type and nonnegative value, and E1 * 2^E2 is representable in the
  result type, then that is the resulting value; otherwise, the
  behavior is undefined.

So if bit 7 of Data.LastLba3 is 1, (Data.LastLba3 << 24) will be out of
the range within int type. The undefined behavior of the signed left shift
will lead to a potential of setting the high 32 bits of
MediaInfo->LastBlock to 1 during the cast from type int to type UINT64
for X64 builds.

This commit will add an explicit UINT32 type cast for Data.LastLba3 to
resolve this issue.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>
This commit is contained in:
Hao Wu 2017-04-12 08:52:51 +08:00
parent 7c115e775b
commit 3778a4dfcd
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c
View File

@ -2140,7 +2140,7 @@ ReadCapacity (
if (MediaInfo->DeviceType == IdeCDROM) {
MediaInfo->LastBlock = (Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0;
MediaInfo->LastBlock = ((UINT32) Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0;
MediaInfo->MediaPresent = TRUE;
//
// Because the user data portion in the sector of the Data CD supported
@ -2161,7 +2161,7 @@ ReadCapacity (
MediaInfo2->MediaPresent = FALSE;
MediaInfo2->LastBlock = 0;
} else {
MediaInfo->LastBlock = (FormatData.LastLba3 << 24) |
MediaInfo->LastBlock = ((UINT32) FormatData.LastLba3 << 24) |
(FormatData.LastLba2 << 16) |
(FormatData.LastLba1 << 8) |
FormatData.LastLba0;