From nobody Thu May 2 04:35:03 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) client-ip=198.145.21.10; envelope-from=edk2-devel-bounces@lists.01.org; helo=ml01.01.org; Authentication-Results: mx.zoho.com; spf=none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) smtp.mailfrom=edk2-devel-bounces@lists.01.org; Return-Path: Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx.zohomail.com with SMTPS id 1497867870393728.894143388673; Mon, 19 Jun 2017 03:24:30 -0700 (PDT) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 293F821A07A94; Mon, 19 Jun 2017 03:23:06 -0700 (PDT) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 E127421A07A94 for ; Mon, 19 Jun 2017 03:23:04 -0700 (PDT) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP; 19 Jun 2017 03:24:26 -0700 Received: from shwdeopenpsi068.ccr.corp.intel.com ([10.239.9.2]) by fmsmga006.fm.intel.com with ESMTP; 19 Jun 2017 03:24:24 -0700 X-Original-To: edk2-devel@lists.01.org X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,360,1493708400"; d="scan'208";a="116599488" From: Star Zeng To: edk2-devel@lists.01.org Date: Mon, 19 Jun 2017 18:24:22 +0800 Message-Id: <1497867862-117352-1-git-send-email-star.zeng@intel.com> X-Mailer: git-send-email 2.7.0.windows.1 Subject: [edk2] [PATCH] MdeModulePkg: Fix use-after-free error in InstallConfigurationTable() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jiewen Yao , Star Zeng , Liming Gao MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" X-ZohoMail: RSF_4 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" From: "Shi, Steven" When installing configuration table and the original gDxeCoreST->ConfigurationTable[] buffer happen to be not big enough to add a new table, the CoreInstallConfigurationTable() enter the branch of line 113 in InstallConfigurationTable.c to free the old gDxeCoreST->ConfigurationTable[] buffer and allocate a new bigger one. The problem happens at line 139 CoreFreePool(), which is to free the old gDxeCoreST->ConfigurationTable[] buffer. The CoreFreePool()'s behavior is to free the buffer firstly, then call the InstallMemoryAttributesTableOnMemoryAllocation (PoolType) to update the EfiRuntimeServices type memory info, the CoreInstallConfigurationTable() will be re-entered by CoreFreePool() in its calling stack, then use-after-free read error will happen at line 59 of InstallConfigurationTable.c and use-after-free write error will happen at line 151 and 152 of InstallConfigurationTable.c. The patch is to update System table to the new table pointer before calling CoreFreePool() to free the old table. The case above is in DxeCore, but not in PiSmmCore. The change in PiSmmCore is to be consistent with DxeCore. Cc: Jiewen Yao Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Steven Shi Signed-off-by: Star Zeng Reviewed-by: Liming Gao Reviewed-by: Steven Shi --- .../Core/Dxe/Misc/InstallConfigurationTable.c | 34 ++++++++++++++++--= ---- .../Core/PiSmmCore/InstallConfigurationTable.c | 34 ++++++++++++++++--= ---- 2 files changed, 50 insertions(+), 18 deletions(-) mode change 100644 =3D> 100755 MdeModulePkg/Core/Dxe/Misc/InstallConfigura= tionTable.c diff --git a/MdeModulePkg/Core/Dxe/Misc/InstallConfigurationTable.c b/MdeMo= dulePkg/Core/Dxe/Misc/InstallConfigurationTable.c old mode 100644 new mode 100755 index e4735db7ba45..dcdeb7f45803 --- a/MdeModulePkg/Core/Dxe/Misc/InstallConfigurationTable.c +++ b/MdeModulePkg/Core/Dxe/Misc/InstallConfigurationTable.c @@ -1,7 +1,7 @@ /** @file UEFI Miscellaneous boot Services InstallConfigurationTable service =20 -Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD = License which accompanies this distribution. The full text of the license may be = found at @@ -42,6 +42,7 @@ CoreInstallConfigurationTable ( { UINTN Index; EFI_CONFIGURATION_TABLE *EfiConfigurationTable; + EFI_CONFIGURATION_TABLE *OldTable; =20 // // If Guid is NULL, then this operation cannot be performed @@ -68,7 +69,7 @@ CoreInstallConfigurationTable ( if (Table !=3D NULL) { // // If Table is not NULL, then this is a modify operation. - // Modify the table enty and return. + // Modify the table entry and return. // gDxeCoreST->ConfigurationTable[Index].VendorTable =3D Table; =20 @@ -134,15 +135,30 @@ CoreInstallConfigurationTable ( ); =20 // - // Free Old Table + // Record the old table pointer. // - CoreFreePool (gDxeCoreST->ConfigurationTable); - } + OldTable =3D gDxeCoreST->ConfigurationTable; =20 - // - // Update System Table - // - gDxeCoreST->ConfigurationTable =3D EfiConfigurationTable; + // + // As the CoreInstallConfigurationTable() may be re-entered by Cor= eFreePool() + // in its calling stack, updating System table to the new table po= inter must + // be done before calling CoreFreePool() to free the old table. + // It can make sure the gDxeCoreST->ConfigurationTable point to th= e new table + // and avoid the errors of use-after-free to the old table by the = reenter of + // CoreInstallConfigurationTable() in CoreFreePool()'s calling sta= ck. + // + gDxeCoreST->ConfigurationTable =3D EfiConfigurationTable; + + // + // Free the old table after updating System Table to the new table= pointer. + // + CoreFreePool (OldTable); + } else { + // + // Update System Table + // + gDxeCoreST->ConfigurationTable =3D EfiConfigurationTable; + } } =20 // diff --git a/MdeModulePkg/Core/PiSmmCore/InstallConfigurationTable.c b/MdeM= odulePkg/Core/PiSmmCore/InstallConfigurationTable.c index b2f6769c109f..2b6eef9a0e3e 100644 --- a/MdeModulePkg/Core/PiSmmCore/InstallConfigurationTable.c +++ b/MdeModulePkg/Core/PiSmmCore/InstallConfigurationTable.c @@ -1,7 +1,7 @@ /** @file System Management System Table Services SmmInstallConfigurationTable ser= vice =20 - Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availa= ble=20 under the terms and conditions of the BSD License which accompanies this=20 distribution. The full text of the license may be found at =20 @@ -46,6 +46,7 @@ SmmInstallConfigurationTable ( { UINTN Index; EFI_CONFIGURATION_TABLE *ConfigurationTable; + EFI_CONFIGURATION_TABLE *OldTable; =20 // // If Guid is NULL, then this operation cannot be performed @@ -72,7 +73,7 @@ SmmInstallConfigurationTable ( if (Table !=3D NULL) { // // If Table is not NULL, then this is a modify operation. - // Modify the table enty and return. + // Modify the table entry and return. // ConfigurationTable[Index].VendorTable =3D Table; return EFI_SUCCESS; @@ -130,15 +131,30 @@ SmmInstallConfigurationTable ( ); =20 // - // Free Old Table + // Record the old table pointer. // - FreePool (gSmmCoreSmst.SmmConfigurationTable); - } + OldTable =3D gSmmCoreSmst.SmmConfigurationTable; =20 - // - // Update System Table - // - gSmmCoreSmst.SmmConfigurationTable =3D ConfigurationTable; + // + // As the SmmInstallConfigurationTable() may be re-entered by Free= Pool() in + // its calling stack, updating System table to the new table point= er must + // be done before calling FreePool() to free the old table. + // It can make sure the gSmmCoreSmst->SmmConfigurationTable point = to the new + // table and avoid the errors of use-after-free to the old table b= y the + // reenter of SmmInstallConfigurationTable() in FreePool()'s calli= ng stack. + // + gSmmCoreSmst.SmmConfigurationTable =3D ConfigurationTable; + + // + // Free the old table after updating System Table to the new table= pointer. + // + FreePool (OldTable); + } else { + // + // Update System Table + // + gSmmCoreSmst.SmmConfigurationTable =3D ConfigurationTable; + } } =20 // --=20 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel