> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao > A > Sent: Monday, August 9, 2021 2:52 PM > To: devel@edk2.groups.io; mhaeu...@posteo.de > Cc: Wang, Jian J <jian.j.w...@intel.com>; Vitaly Cheptsov > <vit9...@protonmail.com> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Consistent > DebugImageInfoTable updates > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > Marvin > > H?user > > Sent: Monday, August 9, 2021 2:16 PM > > To: Wu, Hao A <hao.a...@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Vitaly Cheptsov > > <vit9...@protonmail.com> > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Consistent > > DebugImageInfoTable updates > > > > Good day Hao, > > > > Sorry for the confusion, and you are (rightfully!) not alone. :( I'll > > quote myself from a different patch: > > > > [...] for some reason, none of the other patch series had indices appended. > > I'm sure I can get that fixed shortly, but what to do then, re-send > > the entire bulk? I don't want to spam the list, maybe it is smarter to > > group them by some overview mail this one time? > > > I would suggest to send a V2 series for all the patches (not only limited to > MdeModulePkg) you sent.
Maybe more than 1 patch series. I cannot tell at this moment since there are many patches sent from you. Best Regards, Hao Wu > > Please ensure that patches belong to one series are generated by a single 'git > format-patch' command. > I think doing so will add information like '1/n', '2/n', ..., 'n/n' for the > patches in > one series. > And you may need to create a cover-letter for one patch series to give a brief > summary on the purpose of the series as a whole. > > Also, if you are implementing a new feature or a fix that touches many > modules, I suggest to file a Bugzilla tracker for it: > Feature request: > https://bugzilla.tianocore.org/enter_bug.cgi?product=Tianocore%20Feature > %20Requests > Bugfix: https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2 > > Lastly, you may keep the 'Reviewed-by' tags already received by other > reviewers. > > Best Regards, > Hao Wu > > > > > > Sorry for the disruption! > > > > Best regards, > > Marvin > > > > On 09/08/2021 08:10, Wu, Hao A wrote: > > > Sorry Marvin Häuser, > > > > > > Could you help to confirm that below 9 MdeModulePkg related patches > > > are > > either: > > > * All independent patches > > > * Belong to a patch series that includes all these 9 MdeModulePkg > > > related > > commits > > > * Belong to several independent patch series > > > > > > MdePkg/Base.h: Introduce various alignment-related macros > > > MdeModulePkg/CoreDxe: Mandatory LoadedImage for > > DebugImageInfoTable > > > MdeModulePkg/DxeCore: Fix DebugImageInfoTable size report > > > MdeModulePkg/DxeCore: Use the correct source for fixed load address > > > MdeModulePkg/PiSmmCore: Drop deprecated image profiling > commands > > > MdeModulePkg/CoreDxe: Drop caller-allocated image buffers > > > MdeModulePkg/DxeCore: Drop unnecessary pointer indirection > > > MdeModulePkg/PiSmmIpl: Correct fixed load address bounds check > > > MdeModulePkg/DxeCore: Consistent DebugImageInfoTable updates > > > > > > Best Regards, > > > Hao Wu > > > > > >> -----Original Message----- > > >> From: Marvin Häuser <mhaeu...@posteo.de> > > >> Sent: Monday, August 9, 2021 3:40 AM > > >> To: devel@edk2.groups.io > > >> Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A > > >> <hao.a...@intel.com>; Bi, Dandan <dandan...@intel.com>; Liming Gao > > >> <gaolim...@byosoft.com.cn>; Vitaly Cheptsov > > >> <vit9...@protonmail.com> > > >> Subject: [PATCH] MdeModulePkg/DxeCore: Consistent > > DebugImageInfoTable > > >> updates > > >> > > >> In theory, modifications to the DebugImageInfoTable may cause > > exceptions. > > >> If the exception handler parses the table, this can lead to > > >> subsequent exceptions if the table state is inconsistent. > > >> > > >> Ensure the DebugImageInfoTable remains consistent during > modifications. > > >> This includes: > > >> 1) Free the old table only only after the new table has been published. > > >> Mitigates use-after-free of the old table. > > >> 2) Do not insert an image entry till it is fully initialised. > > >> Entries may be inserted in the live range if an entry was deleted > previously. > > >> Mitigaes the usage of inconsistent entries. > > >> 3) Free the old image entry only after the table has been updated > > >> with the NULL value. Mitigates use-after-free of the old entry. > > >> 4) Set the MODIFIED state before performing any modifications. > > >> > > >> Cc: Jian J Wang <jian.j.w...@intel.com> > > >> Cc: Hao A Wu <hao.a...@intel.com> > > >> Cc: Dandan Bi <dandan...@intel.com> > > >> Cc: Liming Gao <gaolim...@byosoft.com.cn> > > >> Cc: Vitaly Cheptsov <vit9...@protonmail.com> > > >> Signed-off-by: Marvin Häuser <mhaeu...@posteo.de> > > >> --- > > >> MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c | 60 > +++++++++++-- > > ---- > > >> --- > > >> 1 file changed, 34 insertions(+), 26 deletions(-) > > >> > > >> diff --git a/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c > > >> b/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c > > >> index a75d4158280b..7bd970115111 100644 > > >> --- a/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c > > >> +++ b/MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c > > >> @@ -165,10 +165,11 @@ CoreNewDebugImageInfoEntry ( > > >> IN EFI_HANDLE ImageHandle ) {- > > >> EFI_DEBUG_IMAGE_INFO > > >> *Table;- EFI_DEBUG_IMAGE_INFO *NewTable;- UINTN > > Index;- > > >> UINTN TableSize;+ EFI_DEBUG_IMAGE_INFO > > >> *Table;+ > > >> EFI_DEBUG_IMAGE_INFO *NewTable;+ UINTN > > >> Index;+ > > >> UINTN TableSize;+ EFI_DEBUG_IMAGE_INFO_NORMAL > > >> *NormalImage; // // Set the flag indicating that we're in the > > >> process > of > > >> updating the table.@@ -203,14 +204,6 @@ > > CoreNewDebugImageInfoEntry ( > > >> // Copy the old table into the new one // CopyMem > > >> (NewTable, > > Table, > > >> TableSize);- //- // Free the old table- //- CoreFreePool > > >> (Table);- > //- > > >> // Update the table header- //- Table = NewTable; > > >> mDebugInfoTableHeader.EfiDebugImageInfoTable = NewTable; // > // > > >> Enlarge the max table entries and set the first empty entry index > > >> to@@ - > > >> 218,24 +211,34 @@ CoreNewDebugImageInfoEntry ( > > >> // Index = mMaxTableEntries; mMaxTableEntries > > >> += > > >> EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE;+ //+ // Free the > old > > >> table+ //+ CoreFreePool (Table);+ //+ // Update the table > header+ > > >> //+ Table = NewTable; } // // Allocate data for new entry //- > > >> Table[Index].NormalImage = AllocateZeroPool (sizeof > > >> (EFI_DEBUG_IMAGE_INFO_NORMAL));- if > (Table[Index].NormalImage != > > >> NULL) {+ NormalImage = AllocateZeroPool (sizeof > > >> (EFI_DEBUG_IMAGE_INFO_NORMAL));+ if (NormalImage != NULL) > { // > > >> // Update the entry //- Table[Index].NormalImage->ImageInfoType > > >> = (UINT32) ImageInfoType;- Table[Index].NormalImage- > > >>> LoadedImageProtocolInstance = LoadedImage;- > > >> Table[Index].NormalImage->ImageHandle = ImageHandle;+ > > >> NormalImage->ImageInfoType = (UINT32) ImageInfoType;+ > > >> NormalImage->LoadedImageProtocolInstance = LoadedImage;+ > > >> NormalImage->ImageHandle = ImageHandle; //- // > > >> Increase > > the > > >> number of EFI_DEBUG_IMAGE_INFO elements and set the > > >> mDebugInfoTable in modified status.+ // Set the mDebugInfoTable in > > >> modified status, insert the entry, and+ // increase the number of > > >> EFI_DEBUG_IMAGE_INFO elements. //- > > >> mDebugInfoTableHeader.TableSize++; > > >> mDebugInfoTableHeader.UpdateStatus |= > > >> EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED;+ > > Table[Index].NormalImage > > >> = NormalImage;+ mDebugInfoTableHeader.TableSize++; } > > >> mDebugInfoTableHeader.UpdateStatus &= > > >> ~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS; }@@ -253,8 > +256,9 > > @@ > > >> CoreRemoveDebugImageInfoEntry ( > > >> EFI_HANDLE ImageHandle ) {- EFI_DEBUG_IMAGE_INFO *Table;- > > UINTN > > >> Index;+ EFI_DEBUG_IMAGE_INFO *Table;+ UINTN > > Index;+ > > >> EFI_DEBUG_IMAGE_INFO_NORMAL *NormalImage; > > >> mDebugInfoTableHeader.UpdateStatus |= > > >> EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS; @@ -263,16 > +267,20 > > @@ > > >> CoreRemoveDebugImageInfoEntry ( > > >> for (Index = 0; Index < mMaxTableEntries; Index++) { if > > >> (Table[Index].NormalImage != NULL && Table[Index].NormalImage- > > >>> ImageHandle == ImageHandle) { //- // Found a match. Free up > the > > >> record, then NULL the pointer to indicate the slot- // is free.+ > > >> // > > Found a > > >> match. Set the mDebugInfoTable in modified status and NULL the+ // > > >> pointer to indicate the slot is free and. //- CoreFreePool > > >> (Table[Index].NormalImage);+ NormalImage = > > >> Table[Index].NormalImage;+ mDebugInfoTableHeader.UpdateStatus > > |= > > >> EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED; > > Table[Index].NormalImage > > >> = NULL; //- // Decrease the number of EFI_DEBUG_IMAGE_INFO > > >> elements and set the mDebugInfoTable in modified status.+ // > > Decrease > > >> the number of EFI_DEBUG_IMAGE_INFO elements. // > > >> mDebugInfoTableHeader.TableSize--;- > > >> mDebugInfoTableHeader.UpdateStatus |= > > >> EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED;+ //+ // Free up the > > >> record.+ //+ CoreFreePool (NormalImage); break; } > > >> }-- > > >> 2.31.1 > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78928): https://edk2.groups.io/g/devel/message/78928 Mute This Topic: https://groups.io/mt/84754054/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-