From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 59D962034CF8C for ; Wed, 25 Oct 2017 23:16:33 -0700 (PDT) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2017 23:20:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,434,1503385200"; d="scan'208";a="165143890" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga005.jf.intel.com with ESMTP; 25 Oct 2017 23:20:18 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 25 Oct 2017 23:20:18 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 25 Oct 2017 23:20:17 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.218]) with mapi id 14.03.0319.002; Thu, 26 Oct 2017 14:20:15 +0800 From: "Wang, Jian J" To: "edk2-devel@lists.01.org" CC: "Yao, Jiewen" , "Dong, Eric" , Laszlo Ersek , "Ni, Ruiyu" Thread-Topic: [edk2] [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection Thread-Index: AQHTS5kO2hZYajFq9EGF8YhGCJfReqL1rQ9w Date: Thu, 26 Oct 2017 06:20:14 +0000 Message-ID: References: <20171023005054.7528-1-jian.j.wang@intel.com> <20171023005054.7528-6-jian.j.wang@intel.com> In-Reply-To: <20171023005054.7528-6-jian.j.wang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDc2MTcxZmMtZTU3Zi00MGIzLTkzMDEtZDgwZDBjMDk4MDJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJzdjJYODVQODRLdStwUDZyWFwvUjRRRGZ3NVB4RkxxQWpcL04zaU5xT2E1ZFJmWUdTdjB5ZHBYQ3VmQmppVEU3OVkifQ== x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection 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: , X-List-Received-Date: Thu, 26 Oct 2017 06:16:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo and Ruiyu, Could you please take a look at this part? There's a new protocol introduce= d. Thanks, Jian > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ji= an J > Wang > Sent: Monday, October 23, 2017 8:51 AM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Dong, Eric > Subject: [edk2] [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page > table protection >=20 > > v3 > > According to Jiewen's feedback, implement new protocol > > gEdkiiSmmMemoryAttributeProtocolGuid > > to change memory attributes. >=20 > > v2 > > According to Eric's feedback: > > a. Enclose bit-or with parentheses > > b. Add code in 32-bit code to bypass setting page table to read-only >=20 > Heap guard feature will update page attributes frequently. The page table > should not set to be read-only if heap guard feature is enabled for SMM > mode. Otherwise this feature cannot work. >=20 > Cc: Eric Dong > Cc: Jiewen Yao > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 7 + > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 20 +++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 98 > +++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 2 + > UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 163 > +++++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 3 +- > 6 files changed, 292 insertions(+), 1 deletion(-) >=20 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > index f295c2ebf2..27c11f1b8d 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > @@ -184,6 +184,13 @@ SetPageTableAttributes ( > BOOLEAN IsSplitted; > BOOLEAN PageTableSplitted; >=20 > + // > + // Don't mark page table as read-only if heap guard is enabled. > + // > + if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) !=3D 0) { > + return ; > + } > + > DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n")); >=20 > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > index 282d2e6981..8635f2d2c8 100755 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > @@ -76,6 +76,15 @@ EFI_SMM_CPU_PROTOCOL mSmmCpu =3D { > SmmWriteSaveState > }; >=20 > +/// > +/// SMM Memory Attribute Protocol instance > +/// > +EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL mSmmMemoryAttribute =3D { > + EdkiiSmmGetMemoryAttributes, > + EdkiiSmmSetMemoryAttributes, > + EdkiiSmmClearMemoryAttributes > +}; > + > EFI_CPU_INTERRUPT_HANDLER > mExternalVectorTable[EXCEPTION_VECTOR_NUMBER]; >=20 > // > @@ -893,6 +902,17 @@ PiCpuSmmEntry ( > ); > ASSERT_EFI_ERROR (Status); >=20 > + // > + // Install the SMM Memory Attribute Protocol into SMM protocol databas= e > + // > + Status =3D gSmst->SmmInstallProtocolInterface ( > + &mSmmCpuHandle, > + &gEdkiiSmmMemoryAttributeProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &mSmmMemoryAttribute > + ); > + ASSERT_EFI_ERROR (Status); > + > // > // Expose address of CPU Hot Plug Data structure if CPU hot plug is su= pported. > // > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index 1cf85c1481..e1c231e10b 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -25,6 +25,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > #include > #include > #include > +#include >=20 > #include > #include > @@ -1068,4 +1069,101 @@ TransferApToSafeState ( > IN UINTN NumberToFinishAddress > ); >=20 > +/** > + This function set given attributes of the memory region specified by > + BaseAddress and Length. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes The bit mask of attributes to set for the me= mory > + region. > + > + @retval EFI_SUCCESS The attributes were set for the memory r= egion. > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes specified an illegal combinat= ion of > + attributes that cannot be set together. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmSetMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 Attributes > + ); > + > +/** > + This function clears given attributes of the memory region specified b= y > + BaseAddress and Length. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes The bit mask of attributes to set for the me= mory > + region. > + > + @retval EFI_SUCCESS The attributes were set for the memory r= egion. > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes specified an illegal combinat= ion of > + attributes that cannot be set together. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmClearMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 Attributes > + ); > + > +/** > + This function retrieve the attributes of the memory region specified b= y > + BaseAddress and Length. If different attributes are got from different= part > + of the memory region, EFI_NO_MAPPING will be returned. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes Pointer to attributes returned. > + > + @retval EFI_SUCCESS The attributes got for the memory region= . > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes is NULL. > + @retval EFI_NO_MAPPING Attributes are not consistent cross the > memory > + region. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmGetMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 *Attributes > + ); > + > #endif > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index 099792e6ce..43d9edd0d5 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -129,6 +129,7 @@ > gEfiSmmCpuProtocolGuid ## PRODUCES > gEfiSmmReadyToLockProtocolGuid ## NOTIFY > gEfiSmmCpuServiceProtocolGuid ## PRODUCES > + gEdkiiSmmMemoryAttributeProtocolGuid ## PRODUCES >=20 > [Guids] > gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB = # it is > used for S3 boot. > @@ -159,6 +160,7 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ## > CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CO= NSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask = ## > CONSUMES >=20 > [Depex] > gEfiMpServiceProtocolGuid > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > index 3ad5256f1e..eb5e639e4b 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > @@ -1120,3 +1120,166 @@ IsSmmCommBufferForbiddenAddress ( > } > return FALSE; > } > + > +/** > + This function set given attributes of the memory region specified by > + BaseAddress and Length. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes The bit mask of attributes to set for the me= mory > + region. > + > + @retval EFI_SUCCESS The attributes were set for the memory r= egion. > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes specified an illegal combinat= ion of > + attributes that cannot be set together. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmSetMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 Attributes > + ) > +{ > + return SmmSetMemoryAttributes (BaseAddress, Length, Attributes); > +} > + > +/** > + This function clears given attributes of the memory region specified b= y > + BaseAddress and Length. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes The bit mask of attributes to set for the me= mory > + region. > + > + @retval EFI_SUCCESS The attributes were set for the memory r= egion. > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes specified an illegal combinat= ion of > + attributes that cannot be set together. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmClearMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 Attributes > + ) > +{ > + return SmmClearMemoryAttributes (BaseAddress, Length, Attributes); > +} > + > +/** > + This function retrieve the attributes of the memory region specified b= y > + BaseAddress and Length. If different attributes are got from different= part > + of the memory region, EFI_NO_MAPPING will be returned. > + > + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL > instance. > + @param BaseAddress The physical address that is the start addre= ss of > + a memory region. > + @param Length The size in bytes of the memory region. > + @param Attributes Pointer to attributes returned. > + > + @retval EFI_SUCCESS The attributes got for the memory region= . > + @retval EFI_INVALID_PARAMETER Length is zero. > + Attributes is NULL. > + @retval EFI_NO_MAPPING Attributes are not consistent cross the > memory > + region. > + @retval EFI_UNSUPPORTED The processor does not support one or mo= re > + bytes of the memory resource range speci= fied > + by BaseAddress and Length. > + The bit mask of attributes is not suppor= t for > + the memory resource range specified by > + BaseAddress and Length. > + > +**/ > +EFI_STATUS > +EFIAPI > +EdkiiSmmGetMemoryAttributes ( > + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This, > + IN EFI_PHYSICAL_ADDRESS BaseAddress, > + IN UINT64 Length, > + IN UINT64 *Attributes > + ) > +{ > + EFI_PHYSICAL_ADDRESS Address; > + UINT64 *PageEntry; > + UINT64 MemAttr; > + PAGE_ATTRIBUTE PageAttr; > + INT64 Size; > + > + if (Length < SIZE_4KB || Attributes =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Size =3D (INT64)Length; > + MemAttr =3D (UINT64)-1; > + > + do { > + > + PageEntry =3D GetPageTableEntry (BaseAddress, &PageAttr); > + if (PageEntry =3D=3D NULL || PageAttr =3D=3D PageNone) { > + return EFI_UNSUPPORTED; > + } > + > + // > + // If the memory range is cross page table boundary, make sure they > + // share the same attribute. Return EFI_NO_MAPPING if not. > + // > + *Attributes =3D GetAttributesFromPageEntry (PageEntry); > + if (MemAttr !=3D (UINT64)-1 && *Attributes !=3D MemAttr) { > + return EFI_NO_MAPPING; > + } > + > + switch (PageAttr) { > + case Page4K: > + Address =3D *PageEntry & ~mAddressEncMask & > PAGING_4K_ADDRESS_MASK_64; > + Size -=3D (SIZE_4KB - (BaseAddress - Address)); > + BaseAddress +=3D (SIZE_4KB - (BaseAddress - Address)); > + break; > + > + case Page2M: > + Address =3D *PageEntry & ~mAddressEncMask & > PAGING_2M_ADDRESS_MASK_64; > + Size -=3D SIZE_2MB - (BaseAddress - Address); > + BaseAddress +=3D SIZE_2MB - (BaseAddress - Address); > + break; > + > + case Page1G: > + Address =3D *PageEntry & ~mAddressEncMask & > PAGING_1G_ADDRESS_MASK_64; > + Size -=3D SIZE_1GB - (BaseAddress - Address); > + BaseAddress +=3D SIZE_1GB - (BaseAddress - Address); > + break; > + > + default: > + return EFI_UNSUPPORTED; > + } > + > + MemAttr =3D *Attributes; > + > + } while (Size > 0); > + > + return EFI_SUCCESS; > +} > + > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 3dde80f9ba..4d4668a0c6 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -902,7 +902,8 @@ SetPageTableAttributes ( > BOOLEAN IsSplitted; > BOOLEAN PageTableSplitted; >=20 > - if (!mCpuSmmStaticPageTable) { > + if (!mCpuSmmStaticPageTable > + || (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) !=3D 0) { > return ; > } >=20 > -- > 2.14.1.windows.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel