public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Hao Wu <hao.a.wu@intel.com>
To: edk2-devel@lists.01.org, guoheyi@huawei.com
Cc: Hao Wu <hao.a.wu@intel.com>, Feng Tian <feng.tian@intel.com>
Subject: [PATCH v3 4/5] MdeModulePkg/UsbBotPei: Fix undefined behavior in signed left shift
Date: Wed, 12 Apr 2017 09:52:27 +0800	[thread overview]
Message-ID: <20170412015228.20328-5-hao.a.wu@intel.com> (raw)
In-Reply-To: <20170412015228.20328-1-hao.a.wu@intel.com>

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

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

will involve undefined behavior in signed left shift operations.

Since Data.LastLbaX is of type UINT8, they 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
might incur potential issues.

This commit will add an explicit UINT32 type cast for Data.LastLba3 to
refine the codes.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c b/MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c
index eaad626d61..72bde756eb 100644
--- a/MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c
+++ b/MdeModulePkg/Bus/Usb/UsbBotPei/PeiAtapi.c
@@ -1,7 +1,7 @@
 /** @file
 Pei USB ATATPI command implementations.
 
-Copyright (c) 1999 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 1999 - 2017, Intel Corporation. All rights reserved.<BR>
   
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -274,7 +274,7 @@ PeiUsbReadCapacity (
   if (EFI_ERROR (Status)) {
     return EFI_DEVICE_ERROR;
   }
-  LastBlock = (Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0;
+  LastBlock = ((UINT32) Data.LastLba3 << 24) | (Data.LastLba2 << 16) | (Data.LastLba1 << 8) | Data.LastLba0;
 
   if (LastBlock == 0xFFFFFFFF) {
     DEBUG ((EFI_D_INFO, "The usb device LBA count is larger than 0xFFFFFFFF!\n"));
@@ -346,7 +346,7 @@ PeiUsbReadFormattedCapacity (
     PeiBotDevice->Media2.LastBlock     = 0;
 
   } else {
-    LastBlock = (FormatData.LastLba3 << 24) | (FormatData.LastLba2 << 16) | (FormatData.LastLba1 << 8) | FormatData.LastLba0;
+    LastBlock = ((UINT32) FormatData.LastLba3 << 24) | (FormatData.LastLba2 << 16) | (FormatData.LastLba1 << 8) | FormatData.LastLba0;
     if (LastBlock == 0xFFFFFFFF) {
       DEBUG ((EFI_D_INFO, "The usb device LBA count is larger than 0xFFFFFFFF!\n"));
     }
-- 
2.12.0.windows.1



  parent reply	other threads:[~2017-04-12  1:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-12  1:52 [PATCH v3 0/5] Fix undefined behavior in signed left shift Hao Wu
2017-04-12  1:52 ` [PATCH v3 1/5] MdeModulePkg/ScsiDiskDxe: " Hao Wu
2017-04-12  1:52 ` [PATCH v3 2/5] MdeModulePkg/IdeBusPei: " Hao Wu
2017-04-12  1:52 ` [PATCH v3 3/5] MdeModulePkg/UfsBlkIoPei: " Hao Wu
2017-04-12  1:52 ` Hao Wu [this message]
2017-04-12  1:52 ` [PATCH v3 5/5] IntelFWMdlPkg/IdeBusDxe: " Hao Wu
2017-04-12  1:53 ` [PATCH v3 0/5] " Tian, Feng
2017-04-12  1:55   ` Tian, Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170412015228.20328-5-hao.a.wu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox