From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0731.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe46::731]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9B8568209D for ; Wed, 8 Feb 2017 01:38:24 -0800 (PST) Received: from CS1PR84MB0295.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.26) by CS1PR84MB0294.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.888.16; Wed, 8 Feb 2017 09:38:22 +0000 Received: from CS1PR84MB0295.NAMPRD84.PROD.OUTLOOK.COM ([10.162.190.26]) by CS1PR84MB0295.NAMPRD84.PROD.OUTLOOK.COM ([10.162.190.26]) with mapi id 15.01.0888.026; Wed, 8 Feb 2017 09:38:22 +0000 From: "Wang, Sunny (HPS SW)" To: Laszlo Ersek , Ruiyu Ni , "edk2-devel@lists.01.org" CC: Jeff Fan , "Wang, Sunny (HPS SW)" Thread-Topic: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it Thread-Index: AQHSgO2AyU09RFJZvkOgZUTZVkAe46FeYCEggAB2X4CAAASUwA== Date: Wed, 8 Feb 2017 09:38:22 +0000 Message-ID: References: <20170207025345.599532-1-ruiyu.ni@intel.com> <4c038ec5-b8a2-2f47-afd8-a58a349464de@redhat.com> In-Reply-To: <4c038ec5-b8a2-2f47-afd8-a58a349464de@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=sunnywang@hpe.com; x-originating-ip: [15.211.131.4] x-microsoft-exchange-diagnostics: 1; CS1PR84MB0294; 7:fizBIxgGrfjqcJkyWy4MsljUCh57U2T5UMEguw7xzFsARRvOrf15Luj2VCwqyY2eCeCBOWSJEyxMDLive1QT8A9AgXBOOwjjz0HTm3AkOa3adubIpGdUbQBMKKio7L8Oifkv75DryzhW/OfpNliKRSj+wDVqmDEW8eQs14iXpBqXyMy0q/c0R/gJn2cNMF0saGi7EEuryROHEpP0TcoZlcEozDm/PFSxfoWL8NXpD6QZMgpxlagfD4/cV1hB3Pg1Fr6smaryTBrtIoK7w3dO3aXFLXLKNCg+WQwrLJua8mKifj4TpRWgwCHR5DXisL1vEnVPDC0rj8FJQUKQWV7BEWW5V/ccMWgHXTR+ZETLsW0GxdcUjqWWMgobsMYuTxngaKl9LTKzTuNIKhfWas5BwMWaOty7ExLKzgPNSp2DsqOOhtLcqIjSh/a4kvF33tIdqHwSl0ab+/XsS/gRIX7gXjNTGzv/S/BrGVN/l+ems/GNE4YfMytVFydZdvdRrZxkVoiTpbuey3idNY92r9QEEg== x-forefront-antispam-report: SFV:SKI; SCL:-1SFV:NSPM; SFS:(10019020)(6009001)(7916002)(39850400002)(39860400002)(39410400002)(39840400002)(39450400003)(51444003)(199003)(24454002)(13464003)(189002)(377454003)(53936002)(92566002)(2950100002)(2900100001)(66066001)(68736007)(38730400002)(8676002)(81156014)(7696004)(81166006)(8936002)(3660700001)(3280700002)(5660300001)(54356999)(105586002)(122556002)(74316002)(101416001)(54906002)(76176999)(9686003)(86362001)(106356001)(106116001)(7736002)(305945005)(77096006)(6306002)(189998001)(97736004)(33656002)(53546003)(6246003)(6506006)(50986999)(102836003)(2906002)(6436002)(229853002)(3846002)(55016002)(6116002)(4326007)(491001); DIR:OUT; SFP:1102; SCL:1; SRVR:CS1PR84MB0294; H:CS1PR84MB0295.NAMPRD84.PROD.OUTLOOK.COM; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; x-ms-office365-filtering-correlation-id: cb81431f-e91d-4202-8c33-08d45006394b x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081); SRVR:CS1PR84MB0294; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(227479698468861)(162533806227266)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(20170203043)(5005006)(2017020702029)(3002001)(10201501046)(6055026)(6041248)(20161123560025)(20161123555025)(20161123564025)(20161123558025)(20161123562025)(6072148); SRVR:CS1PR84MB0294; BCL:0; PCL:0; RULEID:; SRVR:CS1PR84MB0294; x-forefront-prvs: 0212BDE3BE received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2017 09:38:22.4938 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0294 Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Feb 2017 09:38:25 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yeah, I misread the commit message and had a misunderstanding on what situa= tion BmExpandMediaDevicePath() is called .=20 Ray and Lazlo, thanks for both of you clarifying this. :) Regards, Sunny Wang -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Wednesday, February 08, 2017 5:18 PM To: Wang, Sunny (HPS SW) ; Ruiyu Ni = ; edk2-devel@lists.01.org Cc: Jeff Fan Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Han= dle before using it Importance: High On 02/08/17 09:13, Wang, Sunny (HPS SW) wrote: > Looks good to me.=20 > Reviewed-by: Sunny Wang >=20 > However, I saw the other potential issue below. If you also think the=20 > potential issue is valid, you can fix this in the other patch. > We need to consider the case where MDEPKG_NDEBUG is defined. We're=20 > using ASSERT_EFI_ERROR to handle the error status from > LocateDevicePath() in the following code block, which contains=20 > potential bug that we still use uninitialized Handle for the case=20 > where LocateDevicePath returns error status. >=20 > Status =3D gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid,=20 > &TempDevicePath, &Handle); ASSERT_EFI_ERROR (Status); I think that Ray made a sufficient argument in the commit message why the a= ssertion was correct -- it is indeed an assertion, and not error checking. Since the function is called when the Handle supports BlockIo or SimpleFileSystem, when there is no SimpleFileSystem installed on the Handle, BlockIo should be installed on the Handle. Perhaps the wording could be clarified, as in: ... BlockIo is *guaranteed* to be installed on the Handle. Just my two cents anyway. Laszlo >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Ruiyu Ni > Sent: Tuesday, February 07, 2017 10:54 AM > To: edk2-devel@lists.01.org > Cc: Jeff Fan > Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize=20 > Handle before using it >=20 > BmExpandMediaDevicePath contains a bug that it uses the uninitialized Han= dle. >=20 > Since the function is called when the Handle supports BlockIo or SimpleFi= leSystem, when there is no SimpleFileSystem installed on the Handle, BlockI= o should be installed on the Handle. > The fix initializes the Handle by locating the BlockIo protocol from the = device path. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Jeff Fan > --- > MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) >=20 > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c=20 > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > index 75bd5dc..9f99122 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > @@ -991,9 +991,13 @@ BmExpandMediaDevicePath ( > return FileBuffer; > } > =20 > + Status =3D gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid,=20 > + &TempDevicePath, &Handle); ASSERT_EFI_ERROR (Status); > + > // > // For device boot option only pointing to the removable device=20 > handle, > - // should make sure all its children handles (its child partion or med= ia handles) are created and connected.=20 > + // should make sure all its children handles (its child partion or=20 > + media handles) // are created and connected. > // > gBS->ConnectController (Handle, NULL, NULL, TRUE); > =20 > @@ -1004,8 +1008,6 @@ BmExpandMediaDevicePath ( > // returned. After the Block IO protocol is reinstalled, subsequent > // Block IO read/write will success. > // > - Status =3D gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid,=20 > &TempDevicePath, &Handle); > - ASSERT_EFI_ERROR (Status); > Status =3D gBS->HandleProtocol (Handle, &gEfiBlockIoProtocolGuid, (VOI= D **) &BlockIo); > ASSERT_EFI_ERROR (Status); > Buffer =3D AllocatePool (BlockIo->Media->BlockSize); > -- > 2.9.0.windows.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >=20