From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 616C3820A2 for ; Wed, 8 Feb 2017 01:18:30 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 08 Feb 2017 01:18:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="222726618" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga004.fm.intel.com with ESMTP; 08 Feb 2017 01:18:30 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Feb 2017 01:18:29 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Feb 2017 01:18:29 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0248.002; Wed, 8 Feb 2017 17:18:27 +0800 From: "Ni, Ruiyu" To: "Wang, Sunny (HPS SW)" , "edk2-devel@lists.01.org" CC: "Fan, Jeff" Thread-Topic: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it Thread-Index: AQHSgO2C8o8dPuOhiUyUfSmTzpKySKFePlsAgACYIRA= Date: Wed, 8 Feb 2017 09:18:26 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B89A6A0@SHSMSX104.ccr.corp.intel.com> References: <20170207025345.599532-1-ruiyu.ni@intel.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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:18:30 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sunny, It's impossible for the Handle which doesn't have SimpleFileSystem or Block= Io installed. Thanks/Ray > -----Original Message----- > From: Wang, Sunny (HPS SW) [mailto:sunnywang@hpe.com] > Sent: Wednesday, February 8, 2017 4:13 PM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Fan, Jeff ; Wang, Sunny (HPS SW) > > Subject: RE: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize > Handle before using it >=20 > Looks good to me. > Reviewed-by: Sunny Wang >=20 > However, I saw the other potential issue below. If you also think the > 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 > using ASSERT_EFI_ERROR to handle the error status from LocateDevicePath() > in the following code block, which contains potential bug that we still u= se > uninitialized Handle for the case where LocateDevicePath returns error > status. >=20 > Status =3D gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, > &TempDevicePath, &Handle); ASSERT_EFI_ERROR (Status); >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > 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 > Handle before using it >=20 > BmExpandMediaDevicePath contains a bug that it uses the uninitialized > Handle. >=20 > Since the function is called when the Handle supports BlockIo or > SimpleFileSystem, when there is no SimpleFileSystem installed on the Hand= le, > BlockIo 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 > 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, > + &TempDevicePath, &Handle); ASSERT_EFI_ERROR (Status); > + > // > // For device boot option only pointing to the removable device handle= , > - // should make sure all its children handles (its child partion or med= ia > handles) are created and connected. > + // should make sure all its children handles (its child partion or > + 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, > &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