On 04/11/17 03:29, Hao Wu wrote:
> 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.
>
> Cc: Feng Tian <feng.t...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a...@intel.com>
> ---
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index b5eff25b9b..2289f20152 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -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
> @@ -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;
>
Thank you very much for the update.
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
