From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: dandan.bi@intel.com) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by groups.io with SMTP; Mon, 23 Sep 2019 18:28:31 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Sep 2019 18:28:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,542,1559545200"; d="scan'208";a="363821819" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga005.jf.intel.com with ESMTP; 23 Sep 2019 18:28:31 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 23 Sep 2019 18:28:30 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 23 Sep 2019 18:28:30 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.32]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.195]) with mapi id 14.03.0439.000; Tue, 24 Sep 2019 09:28:28 +0800 From: "Dandan Bi" To: "devel@edk2.groups.io" , Ard Biesheuvel , Leif Lindholm CC: Laszlo Ersek , "Gao, Liming" , "Bi, Dandan" Subject: Re: [edk2-devel] [patch v2 1/5] EmbeddedPkg: Unload image on EFI_SECURITY_VIOLATION Thread-Topic: [edk2-devel] [patch v2 1/5] EmbeddedPkg: Unload image on EFI_SECURITY_VIOLATION Thread-Index: AQHVbc4U5Cy95vSiRUyXOFG+rv++96c6EoxQ Date: Tue, 24 Sep 2019 01:28:27 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB40C5EF1F@SHSMSX104.ccr.corp.intel.com> References: <20190918030557.55256-1-dandan.bi@intel.com> <15C569713949E871.11658@groups.io> In-Reply-To: <15C569713949E871.11658@groups.io> 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: dandan.bi@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi EmbeddedPkg maintainers , Could you help push this patch? Thanks, Dandan > -----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: Leif Lindholm ; Ard Biesheuvel > ; Laszlo Ersek > Subject: [edk2-devel] [patch v2 1/5] EmbeddedPkg: 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 EmbeddedPkg which don't have the policy to defer the > execution of the image. >=20 > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Cc: Laszlo Ersek > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1992 > Signed-off-by: Dandan Bi > Acked-by: Ard Biesheuvel > Acked-by: Laszlo Ersek > --- > .../AndroidFastboot/Arm/BootAndroidBootImg.c | 9 +++++++++ > .../Library/AndroidBootImgLib/AndroidBootImgLib.c | 12 ++++++++++++ > 2 files changed, 21 insertions(+) >=20 > diff --git > a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c > b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c > index 591afbe7cc..fe05878b4b 100644 > --- > a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c > +++ > b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c > @@ -71,10 +71,19 @@ StartEfiApplication ( >=20 > // Load the image from the device path with Boot Services function > Status =3D gBS->LoadImage (TRUE, ParentImageHandle, DevicePath, NULL,= 0, > &ImageHandle); > if (EFI_ERROR (Status)) { > + // > + // 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); > + } > return Status; > } >=20 > // Passed LoadOptions to the EFI Application > if (LoadOptionsSize !=3D 0) { > diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > index d9e7aa7d2b..e1036954ee 100644 > --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > @@ -439,10 +439,22 @@ AndroidBootImgBoot ( > + KernelSize; >=20 > Status =3D gBS->LoadImage (TRUE, gImageHandle, > (EFI_DEVICE_PATH *)&KernelDevicePath, > (VOID*)(UINTN)Kernel, KernelSize, &ImageHand= le); > + if (EFI_ERROR (Status)) { > + // > + // 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); > + } > + return Status; > + } >=20 > // Set kernel arguments > Status =3D gBS->HandleProtocol (ImageHandle, > &gEfiLoadedImageProtocolGuid, > (VOID **) &ImageInfo); > ImageInfo->LoadOptions =3D NewKernelArg; > -- > 2.18.0.windows.1 >=20 >=20 >=20