Zhiguang, An implementation of CopyGuid() could use CopyMem(). Does CopyGuid() also need to be avoided?
Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On > Behalf Of Zhiguang Liu > Sent: Tuesday, June 16, 2020 2:05 AM > To: devel@edk2.groups.io > Cc: Zeng, Star <star.z...@intel.com>; Gao, Liming > <liming....@intel.com>; Wang, Jian J > <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com> > Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: avoid > SMM pointers being leaked by not using CopyMem() > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2002 > > This commit will update the logic in function > SmmVariableGetStatistics() > so that the pointer fields ('Next' and 'Name') in > structure > VARIABLE_INFO_ENTRY will not be copied into the SMM > communication buffer. > > Doing so will prevent SMM pointers address from being > leaked into non-SMM > environment. > > Please note that newly introduced internal function > CopyVarInfoEntry() > will not use CopyMem() to copy the whole > VARIABLE_INFO_ENTRY structure and > then zero out the 'Next' and 'Name' fields. This is for > preventing race > conditions where the pointers value might still be read. > > Cc: Star Zeng <star.z...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Signed-off-by: Hao A Wu <hao.a...@intel.com> > Signed-off-by: Zhiguang Liu <zhiguang....@intel.com> > --- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > | 33 +++++++++++++++++++++++++++++++-- > 1 file changed, 31 insertions(+), 2 deletions(-) > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm > .c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm > .c > index caca5c3241..74e756bc00 100644 > --- > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm > .c > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm > .c > @@ -315,6 +315,35 @@ GetFvbCountAndBuffer ( > } > > > > > > +/** > > + Copy only the meaningful fields of the variable > statistics information from > > + source buffer to the destination buffer. Other fields > are filled with zero. > > + > > + @param[out] DstInfoEntry A pointer to the buffer > of destination variable > > + information entry. > > + @param[in] SrcInfoEntry A pointer to the buffer > of source variable > > + information entry. > > + > > +**/ > > +static > > +VOID > > +CopyVarInfoEntry ( > > + OUT VARIABLE_INFO_ENTRY *DstInfoEntry, > > + IN VARIABLE_INFO_ENTRY *SrcInfoEntry > > + ) > > +{ > > + DstInfoEntry->Next = NULL; > > + DstInfoEntry->Name = NULL; > > + > > + CopyGuid (&DstInfoEntry->VendorGuid, &SrcInfoEntry- > >VendorGuid); > > + DstInfoEntry->Attributes = SrcInfoEntry->Attributes; > > + DstInfoEntry->ReadCount = SrcInfoEntry->ReadCount; > > + DstInfoEntry->WriteCount = SrcInfoEntry->WriteCount; > > + DstInfoEntry->DeleteCount = SrcInfoEntry- > >DeleteCount; > > + DstInfoEntry->CacheCount = SrcInfoEntry->CacheCount; > > + DstInfoEntry->Volatile = SrcInfoEntry->Volatile; > > +} > > + > > /** > > Get the variable statistics information from the > information buffer pointed by gVariableInfo. > > > > @@ -377,7 +406,7 @@ SmmVariableGetStatistics ( > *InfoSize = StatisticsInfoSize; > > return EFI_BUFFER_TOO_SMALL; > > } > > - CopyMem (InfoEntry, VariableInfo, sizeof > (VARIABLE_INFO_ENTRY)); > > + CopyVarInfoEntry (InfoEntry, VariableInfo); > > CopyMem (InfoName, VariableInfo->Name, NameSize); > > *InfoSize = StatisticsInfoSize; > > return EFI_SUCCESS; > > @@ -417,7 +446,7 @@ SmmVariableGetStatistics ( > return EFI_BUFFER_TOO_SMALL; > > } > > > > - CopyMem (InfoEntry, VariableInfo, sizeof > (VARIABLE_INFO_ENTRY)); > > + CopyVarInfoEntry (InfoEntry, VariableInfo); > > CopyMem (InfoName, VariableInfo->Name, NameSize); > > *InfoSize = StatisticsInfoSize; > > > > -- > 2.25.1.windows.1 > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this > group. > > View/Reply Online (#61324): > https://edk2.groups.io/g/devel/message/61324 > Mute This Topic: https://groups.io/mt/74912557/1643496 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [michael.d.kin...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61363): https://edk2.groups.io/g/devel/message/61363 Mute This Topic: https://groups.io/mt/74912557/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
