From d5aef955b917e1afc2a76b68f91154aa77e0e12c Mon Sep 17 00:00:00 2001 From: Star Zeng Date: Thu, 8 Dec 2016 18:16:05 +0800 Subject: MdeModulePkg VariableSmm: Check InfoSize correctly REF: https://bugzilla.tianocore.org/show_bug.cgi?id=290 Current SmmVariableGetStatistics() in VariableSmm.c is always checking input InfoSize against the first variable info, it is incorrect. For instance, there are three variables. BootOrder Boot0000 Boot0001 If the input InfoEntry is holding the second variable info (Boot0000) and InfoSize is sizeof (VARIABLE_INFO_ENTRY) + StrSize (L"Boot0000"), current code will return EFI_BUFFER_TOO_SMALL, but it should return the third variable info (Boot0001). This patch is to refine the code logic. Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Star Zeng Reviewed-by: Jiewen Yao --- .../Universal/Variable/RuntimeDxe/VariableSmm.c | 25 +++++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c index c714916019..2184634f35 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c @@ -349,9 +349,10 @@ SmmVariableGetStatistics ( ) { VARIABLE_INFO_ENTRY *VariableInfo; - UINTN NameLength; + UINTN NameSize; UINTN StatisticsInfoSize; CHAR16 *InfoName; + UINTN InfoNameMaxSize; EFI_GUID VendorGuid; if (InfoEntry == NULL) { @@ -363,12 +364,13 @@ SmmVariableGetStatistics ( return EFI_UNSUPPORTED; } - StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize (VariableInfo->Name); + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY); if (*InfoSize < StatisticsInfoSize) { *InfoSize = StatisticsInfoSize; return EFI_BUFFER_TOO_SMALL; } InfoName = (CHAR16 *)(InfoEntry + 1); + InfoNameMaxSize = (*InfoSize - sizeof (VARIABLE_INFO_ENTRY)); CopyGuid (&VendorGuid, &InfoEntry->VendorGuid); @@ -376,8 +378,14 @@ SmmVariableGetStatistics ( // // Return the first variable info // + NameSize = StrSize (VariableInfo->Name); + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize; + if (*InfoSize < StatisticsInfoSize) { + *InfoSize = StatisticsInfoSize; + return EFI_BUFFER_TOO_SMALL; + } CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY)); - CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name)); + CopyMem (InfoName, VariableInfo->Name, NameSize); *InfoSize = StatisticsInfoSize; return EFI_SUCCESS; } @@ -387,9 +395,9 @@ SmmVariableGetStatistics ( // while (VariableInfo != NULL) { if (CompareGuid (&VariableInfo->VendorGuid, &VendorGuid)) { - NameLength = StrSize (VariableInfo->Name); - if (NameLength == StrSize (InfoName)) { - if (CompareMem (VariableInfo->Name, InfoName, NameLength) == 0) { + NameSize = StrSize (VariableInfo->Name); + if (NameSize <= InfoNameMaxSize) { + if (CompareMem (VariableInfo->Name, InfoName, NameSize) == 0) { // // Find the match one // @@ -409,14 +417,15 @@ SmmVariableGetStatistics ( // // Output the new variable info // - StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + StrSize (VariableInfo->Name); + NameSize = StrSize (VariableInfo->Name); + StatisticsInfoSize = sizeof (VARIABLE_INFO_ENTRY) + NameSize; if (*InfoSize < StatisticsInfoSize) { *InfoSize = StatisticsInfoSize; return EFI_BUFFER_TOO_SMALL; } CopyMem (InfoEntry, VariableInfo, sizeof (VARIABLE_INFO_ENTRY)); - CopyMem (InfoName, VariableInfo->Name, StrSize (VariableInfo->Name)); + CopyMem (InfoName, VariableInfo->Name, NameSize); *InfoSize = StatisticsInfoSize; return EFI_SUCCESS; -- cgit v1.2.3