From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 3910321939222 for ; Mon, 10 Apr 2017 18:40:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1491874844; x=1523410844; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=mYg06l1H6+TqaOP9dcQhuJBq4kd0ivk2VfiVxBihNM0=; b=WVyQIJg3Y9RJ1oAq8wNmXvDSFxUpTYIfFWv1m7vj0Og31fsbOHzht5ns ybzkrLHNz3RtI1CzaXdbpK/zl9DD9w==; Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Apr 2017 18:40:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,184,1488873600"; d="scan'208";a="85860029" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga005.jf.intel.com with ESMTP; 10 Apr 2017 18:40:43 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 10 Apr 2017 18:40:43 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 10 Apr 2017 18:40:42 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.193]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0319.002; Tue, 11 Apr 2017 09:40:41 +0800 From: "Tian, Feng" To: "Wu, Hao A" , "edk2-devel@lists.01.org" , "lersek@redhat.com" , "guoheyi@huawei.com" CC: "Tian, Feng" Thread-Topic: [PATCH v2] MdeModulePkg/ScsiDiskDxe: Fix undefined behavior in signed left shift Thread-Index: AQHSsmMjSwnks41Y2kO8+nnUVRXR76G/ZGYQ Date: Tue, 11 Apr 2017 01:40:40 +0000 Message-ID: <7F1BAD85ADEA444D97065A60D2E97EE5699E39B0@SHSMSX101.ccr.corp.intel.com> References: <20170411012945.16788-1-hao.a.wu@intel.com> In-Reply-To: <20170411012945.16788-1-hao.a.wu@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] MdeModulePkg/ScsiDiskDxe: 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: Tue, 11 Apr 2017 01:40:44 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Feng Tian Thanks Feng -----Original Message----- From: Wu, Hao A=20 Sent: Tuesday, April 11, 2017 9:30 AM To: edk2-devel@lists.01.org; lersek@redhat.com; guoheyi@huawei.com Cc: Wu, Hao A ; Tian, Feng Subject: [PATCH v2] MdeModulePkg/ScsiDiskDxe: Fix undefined behavior in sig= ned left shift In function GetMediaInfo(), the following expression: ScsiDiskDevice->BlkIo.Media->LastBlock =3D (Capacity10->LastLba3 << 24) | (Capacity10->LastLba2 << 16) | (Capacity10->LastLba1 << 8) | Capacity10->LastLba0; will invol= ve 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 ScsiDisk= Device->BlkIo.Media->LastBlock to 1 during the cast from type int to type U= INT64. This commit will add an explicit UINT32 type cast for Capacity10->LastLba3 to resolve this issue. Cc: Feng Tian Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu --- 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/Bu= s/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. =20 -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at @@ -2754,7 +2= 754,7 @@ GetMediaInfo ( UINT8 *Ptr; =20 if (!ScsiDiskDevice->Cdb16Byte) { - ScsiDiskDevice->BlkIo.Media->LastBlock =3D (Capacity10->LastLba3 << 2= 4) | + ScsiDiskDevice->BlkIo.Media->LastBlock =3D ((UINT32)=20 + Capacity10->LastLba3 << 24) | (Capacity10->LastLba2 << 16)= | (Capacity10->LastLba1 << 8) = | Capacity10->LastLba0; -- 2.12.0.windows.1