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.43, mailfrom: ray.ni@intel.com) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by groups.io with SMTP; Thu, 22 Aug 2019 16:10:29 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Aug 2019 16:10:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,418,1559545200"; d="scan'208";a="184044794" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga006.jf.intel.com with ESMTP; 22 Aug 2019 16:10:27 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 22 Aug 2019 16:10:27 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 22 Aug 2019 16:10:26 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.112]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.80]) with mapi id 14.03.0439.000; Fri, 23 Aug 2019 07:10:25 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "Kinney, Michael D" CC: "Justen, Jordan L" , Andrew Fish , Tim Lewis Subject: Re: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Host: Fix image unload regression Thread-Topic: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Host: Fix image unload regression Thread-Index: AQHVWJJsQy6wzselz0uuTC+WxMfTEqcHy3oA Date: Thu, 22 Aug 2019 23:10:25 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C29519C@SHSMSX104.ccr.corp.intel.com> References: <20190822023610.2068-1-michael.d.kinney@intel.com> <20190822023610.2068-2-michael.d.kinney@intel.com> In-Reply-To: <20190822023610.2068-2-michael.d.kinney@intel.com> 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 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Mike, Thanks for fixing this regression issue. I also did a comparison between this and the Nt32 accordingly code. They are almost the same. I also noticed your unit test steps in Bugzilla and the behavior is expect= ed. I agree and also suggest that this fix to be included in the coming stable= tag release. Reviewed-by: Ray Ni > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Michael D > Kinney > Sent: Thursday, August 22, 2019 10:36 AM > To: devel@edk2.groups.io > Cc: Justen, Jordan L ; Ni, Ray ; > Andrew Fish ; Tim Lewis > Subject: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Hos= t: > Fix image unload regression >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D2104 >=20 > When UEFI Applications or UEFI Drivers are unloaded, the > PeCoffLoaderUnloadImageExtraAction() needs to unload the image using > FreeLibrary() if the image was successfully loaded using LoadLibrrayEx()= . >=20 > This is a regression from the Nt32Pkg that supported unloading applicati= ons and > drivers as well as loading the same application or driver multiple times= . >=20 > Cc: Jordan Justen > Cc: Ray Ni > Cc: Andrew Fish > Cc: Tim Lewis > Signed-off-by: Michael D Kinney > --- > EmulatorPkg/Win/Host/WinHost.c | 167 > +++++++++++++++++++++++++++++++-- > 1 file changed, 161 insertions(+), 6 deletions(-) >=20 > diff --git a/EmulatorPkg/Win/Host/WinHost.c > b/EmulatorPkg/Win/Host/WinHost.c index dd52075f98..9c6acac279 100644 > --- a/EmulatorPkg/Win/Host/WinHost.c > +++ b/EmulatorPkg/Win/Host/WinHost.c > @@ -19,6 +19,25 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define SE_TIME_ZONE_NAME TEXT("SeTimeZonePrivilege") > #endif >=20 > +// > +// The growth size for array of module handle entries // #define > +MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE 0x100 > + > +// > +// Module handle entry structure > +// > +typedef struct { > + CHAR8 *PdbPointer; > + VOID *ModHandle; > +} PDB_NAME_TO_MOD_HANDLE; > + > +// > +// An Array to hold the module handles > +// > +PDB_NAME_TO_MOD_HANDLE *mPdbNameModHandleArray =3D NULL; > +UINTN mPdbNameModHandleArraySize =3D 0; > + > // > // Default information about where the FD is located. > // This array gets filled in with information from PcdWinNtFirmwareVol= ume > @@ -840,6 +859,120 @@ Returns: > return Count; > } >=20 > +/** > + Store the ModHandle in an array indexed by the Pdb File name. > + The ModHandle is needed to unload the image. > + @param ImageContext - Input data returned from PE Laoder Library. Use= d to > find the > + .PDB file name of the PE Image. > + @param ModHandle - Returned from LoadLibraryEx() and stored for ca= ll to > + FreeLibrary(). > + @return return EFI_SUCCESS when ModHandle was stored. > +--*/ > +EFI_STATUS > +AddModHandle ( > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext, > + IN VOID *ModHandle > + ) > + > +{ > + UINTN Index; > + PDB_NAME_TO_MOD_HANDLE *Array; > + UINTN PreviousSize; > + PDB_NAME_TO_MOD_HANDLE *TempArray; > + HANDLE Handle; > + UINTN Size; > + > + // > + // Return EFI_ALREADY_STARTED if this DLL has already been loaded // > + Array =3D mPdbNameModHandleArray; for (Index =3D 0; Index < > + mPdbNameModHandleArraySize; Index++, Array++) { > + if (Array->PdbPointer !=3D NULL && Array->ModHandle =3D=3D ModHandl= e) { > + return EFI_ALREADY_STARTED; > + } > + } > + > + Array =3D mPdbNameModHandleArray; > + for (Index =3D 0; Index < mPdbNameModHandleArraySize; Index++, Array+= +) { > + if (Array->PdbPointer =3D=3D NULL) { > + // > + // Make a copy of the stirng and store the ModHandle > + // > + Handle =3D GetProcessHeap (); > + Size =3D AsciiStrLen (ImageContext->PdbPointer) + 1; > + Array->PdbPointer =3D HeapAlloc ( Handle, HEAP_ZERO_MEMORY, Size)= ; > + ASSERT (Array->PdbPointer !=3D NULL); > + > + AsciiStrCpyS (Array->PdbPointer, Size, ImageContext->PdbPointer); > + Array->ModHandle =3D ModHandle; > + return EFI_SUCCESS; > + } > + } > + > + // > + // No free space in mPdbNameModHandleArray so grow it by // > + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE entires. > + // > + PreviousSize =3D mPdbNameModHandleArraySize * sizeof > + (PDB_NAME_TO_MOD_HANDLE); mPdbNameModHandleArraySize +=3D > + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE; > + // > + // re-allocate a new buffer and copy the old values to the new locait= on. > + // > + TempArray =3D HeapAlloc (GetProcessHeap (), > + HEAP_ZERO_MEMORY, > + mPdbNameModHandleArraySize * sizeof > (PDB_NAME_TO_MOD_HANDLE) > + ); > + > + CopyMem ((VOID *) (UINTN) TempArray, (VOID *) > + (UINTN)mPdbNameModHandleArray, PreviousSize); > + > + HeapFree (GetProcessHeap (), 0, mPdbNameModHandleArray); > + > + mPdbNameModHandleArray =3D TempArray; > + if (mPdbNameModHandleArray =3D=3D NULL) { > + ASSERT (FALSE); > + return EFI_OUT_OF_RESOURCES; > + } > + > + return AddModHandle (ImageContext, ModHandle); } > + > +/** > + Return the ModHandle and delete the entry in the array. > + @param ImageContext - Input data returned from PE Laoder Library. U= sed > to find the > + .PDB file name of the PE Image. > + @return > + ModHandle - ModHandle assoicated with ImageContext is returned > + NULL - No ModHandle associated with ImageContext > +**/ > +VOID * > +RemoveModHandle ( > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > + ) > +{ > + UINTN Index; > + PDB_NAME_TO_MOD_HANDLE *Array; > + > + if (ImageContext->PdbPointer =3D=3D NULL) { > + // > + // If no PDB pointer there is no ModHandle so return NULL > + // > + return NULL; > + } > + > + Array =3D mPdbNameModHandleArray; > + for (Index =3D 0; Index < mPdbNameModHandleArraySize; Index++, Array+= +) { > + if ((Array->PdbPointer !=3D NULL) && (AsciiStrCmp(Array->PdbPointer= , > ImageContext->PdbPointer) =3D=3D 0)) { > + // > + // If you find a match return it and delete the entry > + // > + HeapFree (GetProcessHeap (), 0, Array->PdbPointer); > + Array->PdbPointer =3D NULL; > + return Array->ModHandle; > + } > + } > + > + return NULL; > +} >=20 > VOID > EFIAPI > @@ -847,6 +980,7 @@ PeCoffLoaderRelocateImageExtraAction ( > IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > ) > { > + EFI_STATUS Status; > VOID *DllEntryPoint; > CHAR16 *DllFileName; > HMODULE Library; > @@ -855,7 +989,7 @@ PeCoffLoaderRelocateImageExtraAction ( > ASSERT (ImageContext !=3D NULL); > // > // If we load our own PE COFF images the Windows debugger can not sou= rce > - // level debug our code. If a valid PDB pointer exists usw it to loa= d > + // level debug our code. If a valid PDB pointer exists use it to > + load > // the *.dll file as a library using Windows* APIs. This allows > // source level debug. The image is still loaded and relocated > // in the Framework memory space like on a real system (by the code = above), > @@ -913,10 +1047,22 @@ PeCoffLoaderRelocateImageExtraAction ( > } >=20 > if ((Library !=3D NULL) && (DllEntryPoint !=3D NULL)) { > - ImageContext->EntryPoint =3D (EFI_PHYSICAL_ADDRESS) (UINTN) > DllEntryPoint; > - SecPrint ("LoadLibraryEx (%S,\n NULL, > DONT_RESOLVE_DLL_REFERENCES)\n", DllFileName); > + Status =3D AddModHandle (ImageContext, Library); > + if (Status =3D=3D EFI_ALREADY_STARTED) { > + // > + // If the DLL has already been loaded before, then this instanc= e of the DLL > can not be debugged. > + // > + ImageContext->PdbPointer =3D NULL; > + SecPrint ("WARNING: DLL already loaded. No source level debug = %S.\n\r", > DllFileName); > + } else { > + // > + // This DLL is not already loaded, so source level debugging is= supported. > + // > + ImageContext->EntryPoint =3D (EFI_PHYSICAL_ADDRESS) (UINTN) > DllEntryPoint; > + SecPrint ("LoadLibraryEx (\n\r %S,\n\r NULL, > DONT_RESOLVE_DLL_REFERENCES)\n\r", DllFileName); > + } > } else { > - SecPrint ("WARNING: No source level debug %S. \n", DllFileName); > + SecPrint ("WARNING: No source level debug %S. \n\r", > + DllFileName); > } >=20 > free (DllFileName); > @@ -926,13 +1072,22 @@ PeCoffLoaderRelocateImageExtraAction ( VOID > EFIAPI PeCoffLoaderUnloadImageExtraAction ( > - IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > ) > { > + VOID *ModHandle; > + > ASSERT (ImageContext !=3D NULL); > + > + ModHandle =3D RemoveModHandle (ImageContext); if (ModHandle !=3D NUL= L) { > + FreeLibrary (ModHandle); > + SecPrint ("FreeLibrary (\n\r %s)\n\r", ImageContext->PdbPointer); > + } else { > + SecPrint ("WARNING: Unload image without source level debug\n\r"); > + } > } >=20 > - > VOID > _ModuleEntryPoint ( > VOID > -- > 2.21.0.windows.1 >=20 >=20 >=20