From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: zhichao.gao@intel.com) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by groups.io with SMTP; Tue, 17 Sep 2019 22:28:48 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Sep 2019 22:28:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,519,1559545200"; d="scan'208";a="177611024" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga007.jf.intel.com with ESMTP; 17 Sep 2019 22:28:45 -0700 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 17 Sep 2019 22:28:07 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.92]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.235]) with mapi id 14.03.0439.000; Wed, 18 Sep 2019 13:28:05 +0800 From: "Gao, Zhichao" To: "devel@edk2.groups.io" , "Bi, Dandan" CC: "Wang, Jian J" , "Wu, Hao A" , "Ni, Ray" , "Gao, Liming" , "Laszlo Ersek" Subject: Re: [edk2-devel] [patch v2 3/5] MdeModulePkg/UefiBootManager: Unload image on EFI_SECURITY_VIOLATION Thread-Topic: [edk2-devel] [patch v2 3/5] MdeModulePkg/UefiBootManager: Unload image on EFI_SECURITY_VIOLATION Thread-Index: AQHVbc4WKEqAqc/gwkqKH6Kmz8souacw5w/w Date: Wed, 18 Sep 2019 05:28:05 +0000 Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B83B413@SHSMSX101.ccr.corp.intel.com> References: <20190918030557.55256-1-dandan.bi@intel.com> <20190918030557.55256-4-dandan.bi@intel.com> In-Reply-To: <20190918030557.55256-4-dandan.bi@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Zhichao Gao Thanks, Zhichao > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Dandan Bi > Sent: Wednesday, September 18, 2019 11:06 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A = ; > Ni, Ray ; Gao, Zhichao ; Gao, > Liming ; Laszlo Ersek > Subject: [edk2-devel] [patch v2 3/5] MdeModulePkg/UefiBootManager: > Unload image on EFI_SECURITY_VIOLATION >=20 > For the LoadImage() boot service, with EFI_SECURITY_VIOLATION retval, th= e > Image was loaded and an ImageHandle was created with a valid > EFI_LOADED_IMAGE_PROTOCOL, but the image can not be started right now. > This follows UEFI Spec. >=20 > But if the caller of LoadImage() doesn't have the option to defer the > execution of an image, we can not treat EFI_SECURITY_VIOLATION like any > other LoadImage() error, we should unload image for the > EFI_SECURITY_VIOLATION to avoid resource leak. >=20 > This patch is to do error handling for EFI_SECURITY_VIOLATION explicitly= for > the callers in UefiBootManagerLib which don't have the policy to defer t= he > execution of the image. >=20 > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Ray Ni > Cc: Zhichao Gao > Cc: Liming Gao > Cc: Laszlo Ersek > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1992 > Signed-off-by: Dandan Bi > --- > MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 9 +++++++++ > .../Library/UefiBootManagerLib/BmLoadOption.c | 11 ++++++++++- > MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 11 > ++++++++++- > 3 files changed, 29 insertions(+), 2 deletions(-) >=20 > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > index 952033fc82..760d7647b8 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c > @@ -1859,10 +1859,19 @@ EfiBootManagerBoot ( > if (FilePath !=3D NULL) { > FreePool (FilePath); > } >=20 > if (EFI_ERROR (Status)) { > + // > + // With EFI_SECURITY_VIOLATION retval, the Image was loaded and a= n > ImageHandle was created > + // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not > be started right now. > + // If the caller doesn't have the option to defer the execution o= f an > image, we should > + // unload image for the EFI_SECURITY_VIOLATION to avoid resource = leak. > + // > + if (Status =3D=3D EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (ImageHandle); > + } > // > // Report Status Code with the failure status to indicate that th= e failure to > load boot option > // > BmReportLoadFailure > (EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status); > BootOption->Status =3D Status; > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > index 07592f8ebd..af47b787d1 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > @@ -1,9 +1,9 @@ > /** @file > Load option library functions which relate with creating and processi= ng load > options. >=20 > -Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.
> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ >=20 > @@ -1409,10 +1409,19 @@ EfiBootManagerProcessLoadOption ( > FileSize, > &ImageHandle > ); > FreePool (FileBuffer); >=20 > + // > + // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an > ImageHandle was created > + // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not be > started right now. > + // If the caller doesn't have the option to defer the execution of = an image, > we should > + // unload image for the EFI_SECURITY_VIOLATION to avoid resource le= ak. > + // > + if (Status =3D=3D EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (ImageHandle); > + } > if (!EFI_ERROR (Status)) { > Status =3D gBS->HandleProtocol (ImageHandle, > &gEfiLoadedImageProtocolGuid, (VOID **)&ImageInfo); > ASSERT_EFI_ERROR (Status); >=20 > ImageInfo->LoadOptionsSize =3D LoadOption->OptionalDataSize; diff= --git > a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c > b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c > index 6b8fb4d924..833e38c6fe 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c > @@ -1,9 +1,9 @@ > /** @file > Misc library functions. >=20 > -Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ >=20 > @@ -491,10 +491,19 @@ EfiBootManagerDispatchDeferredImages ( > ImageDevicePath, > NULL, > 0, > &ImageHandle > ); > + // > + // With EFI_SECURITY_VIOLATION retval, the Image was loaded and a= n > ImageHandle was created > + // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not > be started right now. > + // If the caller doesn't have the option to defer the execution o= f an > image, we should > + // unload image for the EFI_SECURITY_VIOLATION to avoid resource = leak. > + // > + if (Status =3D=3D EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (ImageHandle); > + } > if (!EFI_ERROR (Status)) { > LoadCount++; > // > // Before calling the image, enable the Watchdog Timer for > // a 5 Minute period > -- > 2.18.0.windows.1 >=20 >=20 >=20