Skip to content

Commit

Permalink
MdeModulePkg/ScsiDiskDxe: Fix undefined behavior in signed left shift
Browse files Browse the repository at this point in the history
In function GetMediaInfo(), the following expression:
ScsiDiskDevice->BlkIo.Media->LastBlock =  (Capacity10->LastLba3 << 24) |
                                          (Capacity10->LastLba2 << 16) |
                                          (Capacity10->LastLba1 << 8)  |
                                           Capacity10->LastLba0;
will involve undefined behavior in signed left shift operations.

Since Capacity10->LastLbaX is of type UINT8, and
ScsiDiskDevice->BlkIo.Media->LastBlock is of type UINT64. Therefore,
Capacity10->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 Capacity10->LastLba3 is 1, (Capacity10->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 ScsiDiskDevice->BlkIo.Media->LastBlock to 1 during the cast from type
int to type UINT64.

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

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <[email protected]>
Reviewed-by: Feng Tian <[email protected]>
Reviewed-by: Laszlo Ersek <[email protected]>
  • Loading branch information
hwu25 committed Apr 13, 2017
1 parent 7a14d54 commit 7c115e7
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @file
SCSI disk driver that layers on every SCSI IO protocol in the system.
Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
Expand Down Expand Up @@ -2754,7 +2754,7 @@ GetMediaInfo (
UINT8 *Ptr;

if (!ScsiDiskDevice->Cdb16Byte) {
ScsiDiskDevice->BlkIo.Media->LastBlock = (Capacity10->LastLba3 << 24) |
ScsiDiskDevice->BlkIo.Media->LastBlock = ((UINT32) Capacity10->LastLba3 << 24) |
(Capacity10->LastLba2 << 16) |
(Capacity10->LastLba1 << 8) |
Capacity10->LastLba0;
Expand Down

0 comments on commit 7c115e7

Please sign in to comment.