Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: Taylor Beebe <taylor.d.be...@gmail.com> > Sent: Thursday, June 16, 2022 5:23 AM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar, > Rahul1 <rahul1.ku...@intel.com> > Subject: [PATCH v1 1/1] UefiCpuPkg: CpuDxe: Set RW and P Attributes on Split > Pages > > From: Taylor Beebe <t...@taylorbeebe.com> > > A memory range can be submitted for attribute changes which is large > enough to not require a page split during the attribute update. Consider > the following scenario: > > 1. An attribute update removed the RW attribute on a range large enough > to not require a page split. > 2. Later, an attributes update is called to re-add the RW attribute for > a subsection of that larger page which requires a split > 3. The attribute update logic performs a page split, so now the parent > and child pages have matching attributes > 4. Then, the attribute update logic changes the child page to have the > RW attribute. > 5. The child page would then correctly have the RW attribute added but > the parent page would still have the RW attribute removed which will > cause an improper access violation. > > The page being split should have loose attributes to accommodate the > above case. The split page should always have the attributes set so > the lowest level page frame determines the access rights as detailed > in 4.10.2.2 of the Intel 64 and IA-32 Architectures Software > Developer Manual. Setting the User/Supervisor attribute shouldn't > be necessary. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Signed-off-by: Taylor Beebe <t...@taylorbeebe.com> > --- > UefiCpuPkg/CpuDxe/CpuPageTable.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c > b/UefiCpuPkg/CpuDxe/CpuPageTable.c > index f7a4d92e921a..288d9996f6c3 100644 > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c > @@ -38,6 +38,8 @@ > #define IA32_PG_NX BIT63 > > #define PAGE_ATTRIBUTE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_U | IA32_PG_RW > | IA32_PG_P) > +#define PAGE_ATTRIBUTE_BITS_POST_SPLIT (IA32_PG_RW | IA32_PG_P) > + > // > // Bits 1, 2, 5, 6 are reserved in the IA32 PAE PDPTE > // X64 PAE PDPTE does not have such restriction > @@ -583,7 +585,7 @@ SplitPage ( > NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) | > AddressEncMask | ((*PageEntry) & > PAGE_PROGATE_BITS); > } > > - (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | > ((*PageEntry) & PAGE_ATTRIBUTE_BITS); > + (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | > PAGE_ATTRIBUTE_BITS_POST_SPLIT; > return RETURN_SUCCESS; > } else { > return RETURN_UNSUPPORTED; > @@ -606,7 +608,7 @@ SplitPage ( > NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) | > AddressEncMask | IA32_PG_PS | ((*PageEntry) & > PAGE_PROGATE_BITS); > } > > - (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | > ((*PageEntry) & PAGE_ATTRIBUTE_BITS); > + (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | > PAGE_ATTRIBUTE_BITS_POST_SPLIT; > return RETURN_SUCCESS; > } else { > return RETURN_UNSUPPORTED; > -- > 2.32.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90705): https://edk2.groups.io/g/devel/message/90705 Mute This Topic: https://groups.io/mt/91786433/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-