From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3F9A181F8F for ; Fri, 9 Dec 2016 00:47:52 -0800 (PST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP; 09 Dec 2016 00:47:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,323,1477983600"; d="scan'208";a="40684057" Received: from shwdeopenpsi068.ccr.corp.intel.com ([10.239.9.9]) by fmsmga005.fm.intel.com with ESMTP; 09 Dec 2016 00:47:50 -0800 From: Star Zeng To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao Date: Fri, 9 Dec 2016 16:47:46 +0800 Message-Id: <1481273266-181628-1-git-send-email-star.zeng@intel.com> X-Mailer: git-send-email 2.7.0.windows.1 Subject: [PATCH] MdeModulePkg VariableSmm: Check InfoSize correctly X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Dec 2016 08:47:52 -0000 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 --- .../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 eafb53322e8c..85158d8b46ae 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; -- 2.7.0.windows.1