From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DD439207E57DD for ; Tue, 11 Apr 2017 18:52:36 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Apr 2017 18:52:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,187,1488873600"; d="scan'208";a="1134232244" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.13]) by fmsmga001.fm.intel.com with ESMTP; 11 Apr 2017 18:52:35 -0700 From: Hao Wu To: edk2-devel@lists.01.org, guoheyi@huawei.com Cc: Hao Wu , Feng Tian Date: Wed, 12 Apr 2017 09:52:26 +0800 Message-Id: <20170412015228.20328-4-hao.a.wu@intel.com> X-Mailer: git-send-email 2.12.0.windows.1 In-Reply-To: <20170412015228.20328-1-hao.a.wu@intel.com> References: <20170412015228.20328-1-hao.a.wu@intel.com> Subject: [PATCH v3 3/5] MdeModulePkg/UfsBlkIoPei: Fix undefined behavior in signed left shift X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Apr 2017 01:52:37 -0000 In function UfsBlockIoPeimGetMediaInfo(), the following expression: Private->Media[DeviceIndex].LastBlock = (Capacity16.LastLba3 << 24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | Capacity16.LastLba0; (There is also a similar case in this function.) will involve undefined behavior in signed left shift operations. Since Capacity16.LastLbaX is of type UINT8, and Private->Media[DeviceIndex].LastBlock is of type UINT64. Therefore, Capacity16.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 Capacity16.LastLba3 is 1, (Capacity16.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 Private->Media[DeviceIndex].LastBlock to 1 during the cast from type int to type UINT64. This commit will add an explicit UINT32 type cast for Capacity16.LastLba3 to resolve this issue. Cc: Feng Tian Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu --- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c index cd7c59edae..ddeee3e1bc 100644 --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c @@ -1,6 +1,6 @@ /** @file - Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.
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 @@ -737,11 +737,11 @@ UfsBlockIoPeimGetMediaInfo ( if (EFI_ERROR (Status)) { return EFI_DEVICE_ERROR; } - Private->Media[DeviceIndex].LastBlock = (Capacity16.LastLba3 << 24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | Capacity16.LastLba0; + Private->Media[DeviceIndex].LastBlock = ((UINT32)Capacity16.LastLba3 << 24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | Capacity16.LastLba0; Private->Media[DeviceIndex].LastBlock |= LShiftU64 ((UINT64)Capacity16.LastLba7, 56) | LShiftU64((UINT64)Capacity16.LastLba6, 48) | LShiftU64 ((UINT64)Capacity16.LastLba5, 40) | LShiftU64 ((UINT64)Capacity16.LastLba4, 32); Private->Media[DeviceIndex].BlockSize = (Capacity16.BlockSize3 << 24) | (Capacity16.BlockSize2 << 16) | (Capacity16.BlockSize1 << 8) | Capacity16.BlockSize0; } else { - Private->Media[DeviceIndex].LastBlock = (Capacity.LastLba3 << 24) | (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0; + Private->Media[DeviceIndex].LastBlock = ((UINT32)Capacity.LastLba3 << 24) | (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0; Private->Media[DeviceIndex].BlockSize = (Capacity.BlockSize3 << 24) | (Capacity.BlockSize2 << 16) | (Capacity.BlockSize1 << 8) | Capacity.BlockSize0; } -- 2.12.0.windows.1