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>

1 files changed, 2 insertions, 2 deletions

diff --git a/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c b/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c
index b1ab34d597..58bef161fb 100644
--- a/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c
+++ b/MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c
@@ -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;