From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (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 5069581EA4 for ; Thu, 8 Dec 2016 22:53:20 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP; 08 Dec 2016 22:53:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,323,1477983600"; d="scan'208";a="796025684" Received: from shwdeopenpsi068.ccr.corp.intel.com ([10.239.9.9]) by FMSMGA003.fm.intel.com with ESMTP; 08 Dec 2016 22:53:18 -0800 From: Star Zeng To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao , Jeff Fan Date: Fri, 9 Dec 2016 14:53:16 +0800 Message-Id: <1481266396-180280-1-git-send-email-star.zeng@intel.com> X-Mailer: git-send-email 2.7.0.windows.1 Subject: [PATCH] MdeModulePkg VariableSmm: Do not need check CommBufferSize buffer 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 06:53:20 -0000 Current code in SmmVariableHandler() checks CommBufferSize buffer to make sure it points to outside SMRAM in "case SMM_VARIABLE_FUNCTION_GET_STATISTICS". But after eaae7b33b1cf6b9f21db1636f219c2b6a8d88afd, CommBufferSize buffer points to SMRAM that was used by SMM core to cache CommSize from SmmCommunication protocol, then the check will fail definitely and GET_STATISTICS feature breaks. In fact, do not need check CommBufferSize buffer at all even before eaae7b33b1cf6b9f21db1636f219c2b6a8d88afd. Before eaae7b33b1cf6b9f21db1636f219c2b6a8d88afd, CommBufferSize buffer pointed to gSmmCorePrivate->BufferSize that is outside SMRAM, the check will success definitely; after eaae7b33b1cf6b9f21db1636f219c2b6a8d88afd, CommBufferSize buffer points to local variable BufferSize (in SMRAM) in SmmEntryPoint(), the check is not needed definitely. The patch is to remove the check. Cc: Jiewen Yao Cc: Jeff Fan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Star Zeng --- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c index eafb53322e8c..c714916019ef 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c @@ -695,11 +695,10 @@ SmmVariableHandler ( // It is covered by previous CommBuffer check // - if (!SmmIsBufferOutsideSmmValid ((EFI_PHYSICAL_ADDRESS)(UINTN)CommBufferSize, sizeof(UINTN))) { - DEBUG ((EFI_D_ERROR, "GetStatistics: SMM communication buffer in SMRAM!\n")); - Status = EFI_ACCESS_DENIED; - goto EXIT; - } + // + // Do not need to check CommBufferSize buffer as it should point to SMRAM + // that was used by SMM core to cache CommSize from SmmCommunication protocol. + // Status = SmmVariableGetStatistics (VariableInfo, &InfoSize); *CommBufferSize = InfoSize + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE; -- 2.7.0.windows.1