From nobody Wed May 15 09:24:55 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+110619+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+110619+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1699013653; cv=none; d=zohomail.com; s=zohoarc; b=EY1kJKgaMbd8QkfOSVg2cKREnkmSkh1PP3qveRmbhZx4WKqPIzpemSFr3Ym8rr5BRw2VQug8DM+LMxAHULtobS00Vi6+ucI13OJcdppAsFL0oor3OR6NHJ1pTWryDagBl2T0QgAL2mlX62jodAlYErjfnEzixIgWKvqRov3LO6M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1699013653; h=Cc:Cc:Date:Date:From:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:Message-ID:Reply-To:Reply-To:Sender:Subject:Subject:To:To:Message-Id; bh=PkN5phLuIe38ZuDaGfys4V4k5+UhiUq1cqM0DlYBlQg=; b=O6gk1Z5eMY2g5/uiKyi+IUB2f2PSAV8mhiQH5jJVpGwkJrEyl+zOEY2l0HsrPpfUg4BkffynG2nPQEvn10bJvMnTAp+4r3o4kdRK3gMwgx6QqX5aqYGRZA2jnXAgDM5N843MkzXkce2Bx6QQNK3vBHiuCLAJPBy1sjUgaFGAq5E= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+110619+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1699013653820468.36867662022064; Fri, 3 Nov 2023 05:14:13 -0700 (PDT) Return-Path: DKIM-Signature: a=rsa-sha256; bh=sYBOyCG3VRPhuYyhQnL63l/UBqkHJIcYGIe/K8n6I5w=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe; s=20140610; t=1699013653; v=1; b=dz3XbYzEAsyxHlw1auSfCdqKYOQSgKzVd3rMI9x4OrAp2phFLl5FpGLxlqCdrPMInFHYCIBZ Ew17cNycz4eKe1cFczNKEXZveYSiy0qqvpv+HQTVb6mAroQf6T7U7N++zKomsO9+Ns8r/JjDIw3 aNv6eS/ymJavwLrMfcvZ9eW0= X-Received: by 127.0.0.2 with SMTP id U5IAYY1788612xmqEzSDlj1h; Fri, 03 Nov 2023 05:14:13 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web10.50330.1699013652089396400 for ; Fri, 03 Nov 2023 05:14:12 -0700 X-IronPort-AV: E=McAfee;i="6600,9927,10882"; a="453230469" X-IronPort-AV: E=Sophos;i="6.03,273,1694761200"; d="scan'208";a="453230469" X-Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Nov 2023 05:14:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10882"; a="1008800642" X-IronPort-AV: E=Sophos;i="6.03,273,1694761200"; d="scan'208";a="1008800642" X-Received: from sh1gapp1009.ccr.corp.intel.com ([10.239.189.219]) by fmsmga006.fm.intel.com with ESMTP; 03 Nov 2023 05:14:09 -0700 From: "Wu, Jiaxin" To: devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Zeng Star , Gerd Hoffmann , Rahul Kumar Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/PiSmmCpuDxeSmm: Fix CP Exception when CET enable Date: Fri, 3 Nov 2023 20:14:05 +0800 Message-Id: <20231103121405.3356-1-jiaxin.wu@intel.com> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,jiaxin.wu@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: hBPQ7CXtU8kdYLf3BrQBzfokx1787277AA= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1699013654802100001 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Shadow stack will stop update after CET disable (DisableCet in DisableReadOnlyPageWriteProtect), but normal smi stack will be continue updated with the function return and enter (DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect), thus leading stack mismatch after CET re-enabled (EnableCet in EnableReadOnlyPageWriteProtect). Normal smi stack and shadow stack must be matched when CET enable, otherwise CP Exception will happen, which is caused by a near RET instruction (See SDM Vol 3, 6.15-Control Protection Exception). With above requirement, CET feature enable & disable must be in the same function to avoid stack mismatch issue. Cc: Eric Dong Cc: Ray Ni Cc: Zeng Star Cc: Gerd Hoffmann Cc: Rahul Kumar Signed-off-by: Jiaxin Wu --- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 10 +- UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 104 ++++++++++++++---= ---- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 19 +++- 3 files changed, 91 insertions(+), 42 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmC= puDxeSmm/PiSmmCpuDxeSmm.h index 654935dc76..daa843b057 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -1554,26 +1554,24 @@ SmmWaitForApArrival ( =20 /** Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1. =20 @param[out] WpEnabled If Cr0.WP is enabled. - @param[out] CetEnabled If CET is enabled. + **/ VOID DisableReadOnlyPageWriteProtect ( - OUT BOOLEAN *WpEnabled, - OUT BOOLEAN *CetEnabled + OUT BOOLEAN *WpEnabled ); =20 /** Enable Write Protect on pages marked as read-only. =20 @param[out] WpEnabled If Cr0.WP should be enabled. - @param[out] CetEnabled If CET should be enabled. + **/ VOID EnableReadOnlyPageWriteProtect ( - BOOLEAN WpEnabled, - BOOLEAN CetEnabled + BOOLEAN WpEnabled ); =20 #endif diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPk= g/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c index 6f49866615..2c198a161a 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c @@ -42,61 +42,44 @@ BOOLEAN mIsReadOnlyPageTable =3D FALSE; =20 /** Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1. =20 @param[out] WpEnabled If Cr0.WP is enabled. - @param[out] CetEnabled If CET is enabled. + **/ VOID DisableReadOnlyPageWriteProtect ( - OUT BOOLEAN *WpEnabled, - OUT BOOLEAN *CetEnabled + OUT BOOLEAN *WpEnabled ) { IA32_CR0 Cr0; =20 - *CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALSE; - Cr0.UintN =3D AsmReadCr0 (); - *WpEnabled =3D (Cr0.Bits.WP !=3D 0) ? TRUE : FALSE; + Cr0.UintN =3D AsmReadCr0 (); + *WpEnabled =3D (Cr0.Bits.WP !=3D 0) ? TRUE : FALSE; if (*WpEnabled) { - if (*CetEnabled) { - // - // CET must be disabled if WP is disabled. Disable CET before cleari= ng CR0.WP. - // - DisableCet (); - } - Cr0.Bits.WP =3D 0; AsmWriteCr0 (Cr0.UintN); } } =20 /** Enable Write Protect on pages marked as read-only. =20 @param[out] WpEnabled If Cr0.WP should be enabled. - @param[out] CetEnabled If CET should be enabled. + **/ VOID EnableReadOnlyPageWriteProtect ( - BOOLEAN WpEnabled, - BOOLEAN CetEnabled + BOOLEAN WpEnabled ) { IA32_CR0 Cr0; =20 if (WpEnabled) { Cr0.UintN =3D AsmReadCr0 (); Cr0.Bits.WP =3D 1; AsmWriteCr0 (Cr0.UintN); - - if (CetEnabled) { - // - // re-enable CET. - // - EnableCet (); - } } } =20 /** Initialize a buffer pool for page table use only. @@ -157,13 +140,29 @@ InitializePageTablePool ( =20 // // If page table memory has been marked as RO, mark the new pool pages a= s read-only. // if (mIsReadOnlyPageTable) { - DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled); + // + // CET must be disabled if WP is disabled. + // + CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALS= E; + if (CetEnabled) { + DisableCet (); + } + + DisableReadOnlyPageWriteProtect (&WpEnabled); + SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, EFI_PAGES= _TO_SIZE (PoolPages), EFI_MEMORY_RO); - EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled); + + // + // Enable the WP and restore CET to enable + // + EnableReadOnlyPageWriteProtect (WpEnabled); + if (CetEnabled) { + EnableCet (); + } } =20 return TRUE; } =20 @@ -1055,11 +1054,19 @@ SetMemMapAttributes ( Status =3D PageTableParse (PageTable, mPagingMode, Map, &Count); } =20 ASSERT_RETURN_ERROR (Status); =20 - DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled); + // + // CET must be disabled if WP is disabled. + // + CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALSE; + if (CetEnabled) { + DisableCet (); + } + + DisableReadOnlyPageWriteProtect (&WpEnabled); =20 MemoryMap =3D MemoryMapStart; for (Index =3D 0; Index < MemoryMapEntryCount; Index++) { DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", M= emoryMap->PhysicalStart, MemoryMap->NumberOfPages)); if (MemoryMap->Type =3D=3D EfiRuntimeServicesCode) { @@ -1085,11 +1092,18 @@ SetMemMapAttributes ( ); =20 MemoryMap =3D NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize); } =20 - EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled); + // + // Enable the WP and restore CET to enable + // + EnableReadOnlyPageWriteProtect (WpEnabled); + if (CetEnabled) { + EnableCet (); + } + FreePool (Map); =20 PatchSmmSaveStateMap (); PatchGdtIdtMap (); =20 @@ -1399,11 +1413,19 @@ SetUefiMemMapAttributes ( =20 PERF_FUNCTION_BEGIN (); =20 DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n")); =20 - DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled); + // + // CET must be disabled if WP is disabled. + // + CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALSE; + if (CetEnabled) { + DisableCet (); + } + + DisableReadOnlyPageWriteProtect (&WpEnabled); =20 if (mUefiMemoryMap !=3D NULL) { MemoryMapEntryCount =3D mUefiMemoryMapSize/mUefiDescriptorSize; MemoryMap =3D mUefiMemoryMap; for (Index =3D 0; Index < MemoryMapEntryCount; Index++) { @@ -1479,11 +1501,17 @@ SetUefiMemMapAttributes ( =20 Entry =3D NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable-= >DescriptorSize); } } =20 - EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled); + // + // Enable the WP and restore CET to enable + // + EnableReadOnlyPageWriteProtect (WpEnabled); + if (CetEnabled) { + EnableCet (); + } =20 // // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCo= mmBufferForbiddenAddress(). // =20 @@ -1881,23 +1909,31 @@ SetPageTableAttributes ( =20 PERF_FUNCTION_BEGIN (); DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n")); =20 // - // Disable write protection, because we need mark page table to be write= protected. - // We need *write* page table memory, to mark itself to be *read only*. + // CET must be disabled if WP is disabled. // - DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled); + CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALSE; + if (CetEnabled) { + DisableCet (); + } + + DisableReadOnlyPageWriteProtect (&WpEnabled); =20 // Set memory used by page table as Read Only. DEBUG ((DEBUG_INFO, "Start...\n")); EnablePageTableProtection (); =20 // - // Enable write protection, after page table attribute updated. + // Enable the WP and restore CET to enable // - EnableReadOnlyPageWriteProtect (TRUE, CetEnabled); + EnableReadOnlyPageWriteProtect (WpEnabled); + if (CetEnabled) { + EnableCet (); + } + mIsReadOnlyPageTable =3D TRUE; =20 // // Flush TLB after mark all page table pool as read only. // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDx= eSmm/SmmProfile.c index 7ac3c66f91..71fdf5f34a 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c @@ -604,11 +604,20 @@ InitPaging ( Limit =3D BASE_4GB; } else { Limit =3D (IsRestrictedMemoryAccess ()) ? LShiftU64 (1, mPhysicalAddre= ssBits) : BASE_4GB; } =20 - DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled); + // + // CET must be disabled if WP is disabled. + // + CetEnabled =3D ((AsmReadCr4 () & CR4_CET_ENABLE) !=3D 0) ? TRUE : FALSE; + if (CetEnabled) { + DisableCet (); + } + + DisableReadOnlyPageWriteProtect (&WpEnabled); + // // [0, 4k] may be non-present. // PreviousAddress =3D ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BI= T1) !=3D 0) ? BASE_4KB : 0; =20 @@ -670,11 +679,17 @@ InitPaging ( // Status =3D ConvertMemoryPageAttributes (PageTable, mPagingMode, Previo= usAddress, Limit - PreviousAddress, MemoryAttrMask, TRUE, NULL); ASSERT_RETURN_ERROR (Status); } =20 - EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled); + // + // Enable the WP and restore CET to enable + // + EnableReadOnlyPageWriteProtect (WpEnabled); + if (CetEnabled) { + EnableCet (); + } =20 // // Flush TLB // CpuFlushTlb (); --=20 2.16.2.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110619): https://edk2.groups.io/g/devel/message/110619 Mute This Topic: https://groups.io/mt/102362300/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-