diff options
author | Hao Wu <hao.a.wu@intel.com> | 2017-04-12 08:52:51 +0800 |
---|---|---|
committer | Hao Wu <hao.a.wu@intel.com> | 2017-04-13 09:10:48 +0800 |
commit | bfbcd7e4b24458939df696ca1faede488dd9daae (patch) | |
tree | b1fd05588b25dfee51e309e91df6fdca36fad7ce | |
parent | 1e2d9309510de4b4337a2d842457b5c55717db29 (diff) | |
download | edk2-platforms-bfbcd7e4b24458939df696ca1faede488dd9daae.tar.xz |
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>
(cherry picked from commit 3778a4dfcd8d8286de3eed6fb5e33871854879e5)
-rw-r--r-- | MdeModulePkg/Bus/Pci/IdeBusPei/AtapiPeim.c | 4 |
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;
|