Reviewed-by: Zhichao Gao <[email protected]> > -----Original Message----- > From: Bi, Dandan > Sent: Wednesday, September 4, 2019 4:26 PM > To: [email protected] > Cc: Ni, Ray <[email protected]>; Gao, Zhichao <[email protected]>; > Laszlo Ersek <[email protected]> > Subject: [patch 3/3] ShellPkg: Unload image on EFI_SECURITY_VIOLATION > > For the LoadImage() boot service, 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. > This follows UEFI Spec. > > 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. > > This patch is to do error handling for EFI_SECURITY_VIOLATION explicitly for > the callers in ShellPkg which don't have the policy to defer the execution of > the image. > > Cc: Ray Ni <[email protected]> > Cc: Zhichao Gao <[email protected]> > Cc: Laszlo Ersek <[email protected]> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1992 > Signed-off-by: Dandan Bi <[email protected]> > --- > ShellPkg/Application/Shell/ShellManParser.c | 9 +++++++++ > .../Library/UefiShellDebug1CommandsLib/LoadPciRom.c | 11 ++++++++++- > ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c | 11 ++++++++++- > 3 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/ShellPkg/Application/Shell/ShellManParser.c > b/ShellPkg/Application/Shell/ShellManParser.c > index 6909f29441..e5f97bbb11 100644 > --- a/ShellPkg/Application/Shell/ShellManParser.c > +++ b/ShellPkg/Application/Shell/ShellManParser.c > @@ -643,10 +643,19 @@ ProcessManFile( > goto Done; > } > DevPath = ShellInfoObject.NewEfiShellProtocol- > >GetDevicePathFromFilePath(CmdFilePathName); > Status = gBS->LoadImage(FALSE, gImageHandle, DevPath, NULL, 0, > &CmdFileImgHandle); > 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 the resource > leak. > + // > + if (Status == EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (CmdFileImgHandle); > + } > *HelpText = NULL; > goto Done; > } > Status = gBS->OpenProtocol( > CmdFileImgHandle, > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/LoadPciRom.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/LoadPciRom.c > index 1b169d0d3c..f91e3eb6e7 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/LoadPciRom.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/LoadPciRom.c > @@ -1,10 +1,10 @@ > /** @file > Main file for LoadPciRom shell Debug1 function. > > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR> > - Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2005 - 2019, Intel Corporation. All rights > + reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > #include "UefiShellDebug1CommandsLib.h" > @@ -332,10 +332,19 @@ LoadEfiDriversFromRomImage ( > ImageBuffer, > ImageLength, > &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 > leak. > + // > + if (Status == EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (ImageHandle); > + } > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN > (STR_LOADPCIROM_LOAD_FAIL), gShellDebug1HiiHandle, L"loadpcirom", > FileName, ImageIndex); > // PrintToken (STRING_TOKEN > (STR_LOADPCIROM_LOAD_IMAGE_ERROR), HiiHandle, ImageIndex, Status); > } else { > Status = gBS->StartImage (ImageHandle, NULL, NULL); > if (EFI_ERROR (Status)) { > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > index 6a94b48c86..a13e1bda2d 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Load.c > @@ -1,10 +1,10 @@ > /** @file > Main file for attrib shell level 2 function. > > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR> > - Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2009 - 2019, Intel Corporation. All rights > + reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > #include "UefiShellLevel2CommandsLib.h" > @@ -110,10 +110,19 @@ LoadDriver( > NULL, > 0, > &LoadedDriverHandle); > > 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 leak. > + // > + if (Status == EFI_SECURITY_VIOLATION) { > + gBS->UnloadImage (LoadedDriverHandle); > + } > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_LOAD_NOT_IMAGE), > gShellLevel2HiiHandle, FileName, Status); > } else { > // > // Make sure it is a driver image > // > -- > 2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46858): https://edk2.groups.io/g/devel/message/46858 Mute This Topic: https://groups.io/mt/33136047/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
