On Tue, Jun 14, 2022 at 03:39:33PM +0900, Hyeonggon Yoo wrote:
> commit a8aed3e0752b4 ("x86/mm/pageattr: Prevent PSE and GLOABL leftovers
> to confuse pmd/pte_present and pmd_huge") made CPA clear _PAGE_GLOBAL when
> _PAGE_PRESENT is not set. This prevents kernel crashing when kernel reads
> a page with !_PAGE_PRESENT and _PAGE_PROTNONE (_PAGE_GLOBAL). And then it
> set _PAGE_GLOBAL back when setting _PAGE_PRESENT again.
>
> After commit d1440b23c922d ("x86/mm: Factor out pageattr _PAGE_GLOBAL
> setting") made kernel not set unconditionally _PAGE_GLOBAL, pages lose
> global flag after _set_pages_np() and _set_pages_p() are called.
>
> But after commit 3166851142411 ("x86: skip check for spurious faults for
> non-present faults"), spurious_kernel_fault() does not confuse
> pte/pmd entries with _PAGE_PROTNONE as present anymore. So simply
> drop pgprot_clear_protnone_bits().
Looks like I forgot to Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
Plus I did check that kernel does not crash when reading from/writing to
non-present pages with this patch applied.
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx>
> ---
> arch/x86/mm/pat/set_memory.c | 24 ------------------------
> 1 file changed, 24 deletions(-)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 67cf969fed0d..8a8ce8d78694 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -746,23 +746,6 @@ static void __set_pmd_pte(pte_t *kpte, unsigned long address, pte_t pte)
> #endif
> }
>
> -static pgprot_t pgprot_clear_protnone_bits(pgprot_t prot)
> -{
> - /*
> - * _PAGE_GLOBAL means "global page" for present PTEs.
> - * But, it is also used to indicate _PAGE_PROTNONE
> - * for non-present PTEs.
> - *
> - * This ensures that a _PAGE_GLOBAL PTE going from
> - * present to non-present is not confused as
> - * _PAGE_PROTNONE.
> - */
> - if (!(pgprot_val(prot) & _PAGE_PRESENT))
> - pgprot_val(prot) &= ~_PAGE_GLOBAL;
> -
> - return prot;
> -}
> -
> static int __should_split_large_page(pte_t *kpte, unsigned long address,
> struct cpa_data *cpa)
> {
> @@ -824,7 +807,6 @@ static int __should_split_large_page(pte_t *kpte, unsigned long address,
> * different bit positions in the two formats.
> */
> req_prot = pgprot_4k_2_large(req_prot);
> - req_prot = pgprot_clear_protnone_bits(req_prot);
> if (pgprot_val(req_prot) & _PAGE_PRESENT)
> pgprot_val(req_prot) |= _PAGE_PSE;
>
> @@ -1013,8 +995,6 @@ __split_large_page(struct cpa_data *cpa, pte_t *kpte, unsigned long address,
> return 1;
> }
>
> - ref_prot = pgprot_clear_protnone_bits(ref_prot);
> -
> /*
> * Get the target pfn from the original entry:
> */
> @@ -1246,8 +1226,6 @@ static void populate_pte(struct cpa_data *cpa,
>
> pte = pte_offset_kernel(pmd, start);
>
> - pgprot = pgprot_clear_protnone_bits(pgprot);
> -
> while (num_pages-- && start < end) {
> set_pte(pte, pfn_pte(cpa->pfn, pgprot));
>
> @@ -1542,8 +1520,6 @@ static int __change_page_attr(struct cpa_data *cpa, int primary)
> new_prot = static_protections(new_prot, address, pfn, 1, 0,
> CPA_PROTECT);
>
> - new_prot = pgprot_clear_protnone_bits(new_prot);
> -
> /*
> * We need to keep the pfn from the existing PTE,
> * after all we're only going to change it's attributes
> --
> 2.32.0
>
--
Thanks,
Hyeonggon
