From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 26 Aug 2019 10:16:28 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7BF13190C01C; Mon, 26 Aug 2019 17:16:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-34.ams2.redhat.com [10.36.117.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 56B7060BFB; Mon, 26 Aug 2019 17:16:27 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 3/5] UefiCpuPkg/PiSmmCpu: Restrict access per PcdCpuSmmRestrictedMemoryAccess To: devel@edk2.groups.io, ray.ni@intel.com Cc: Eric Dong , Jiewen Yao References: <20190825224513.171572-1-ray.ni@intel.com> <20190825224513.171572-4-ray.ni@intel.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 26 Aug 2019 19:16:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190825224513.171572-4-ray.ni@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.70]); Mon, 26 Aug 2019 17:16:28 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/26/19 00:45, Ni, Ray wrote: > Today's behavior is to always restrict access to non-SMRAM regardless > the value of PcdCpuSmmRestrictedMemoryAccess. > > Because RAS components require to access all non-SMRAM memory, the > patch changes the code logic to honor PcdCpuSmmRestrictedMemoryAccess > so that only when the PCD is true, the restriction takes affect and > page table memory is also protected. > > Because IA32 build doesn't reference this PCD, such restriction > always takes affect in IA32 build. > > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Jiewen Yao > Cc: Laszlo Ersek > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 14 ++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 18 ++++++++++-------- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 11 +++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 14 ++++++++++++++ > 4 files changed, 49 insertions(+), 8 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > index 05fb455936..f891a81112 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c > @@ -336,3 +336,17 @@ RestoreCr2 ( > { > return ; > } > + > +/** > + Return whether access to non-SMRAM is restricted. > + > + @retval TRUE Access to non-SMRAM is restricted. > + @retval FALSE Access to non-SMRAM is not restricted. > +*/ > +BOOLEAN > +IsRestrictedMemoryAccess ( > + VOID > + ) > +{ > + return TRUE; > +} > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > index 69a04dfb23..723fd5042f 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > @@ -1431,15 +1431,17 @@ PerformRemainingTasks ( > // > SetMemMapAttributes (); > > - // > - // For outside SMRAM, we only map SMM communication buffer or MMIO. > - // > - SetUefiMemMapAttributes (); > + if (IsRestrictedMemoryAccess ()) { > + // > + // For outside SMRAM, we only map SMM communication buffer or MMIO. > + // > + SetUefiMemMapAttributes (); > > - // > - // Set page table itself to be read-only > - // > - SetPageTableAttributes (); > + // > + // Set page table itself to be read-only > + // > + SetPageTableAttributes (); > + } > > // > // Configure SMM Code Access Check feature if available. > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index 8c29f1a558..daf977f654 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -1450,4 +1450,15 @@ InitializeDataForMmMp ( > VOID > ); > > +/** > + Return whether access to non-SMRAM is restricted. > + > + @retval TRUE Access to non-SMRAM is restricted. > + @retval FALSE Access to non-SMRAM is not restricted. > +*/ > +BOOLEAN > +IsRestrictedMemoryAccess ( > + VOID > + ); > + > #endif > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index 7516f35055..733d107efd 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -1252,3 +1252,17 @@ RestoreCr2 ( > AsmWriteCr2 (Cr2); > } > } > + > +/** > + Return whether access to non-SMRAM is restricted. > + > + @retval TRUE Access to non-SMRAM is restricted. > + @retval FALSE Access to non-SMRAM is not restricted. > +*/ > +BOOLEAN > +IsRestrictedMemoryAccess ( > + VOID > + ) > +{ > + return mCpuSmmRestrictedMemoryAccess; > +} > Reviewed-by: Laszlo Ersek