Re: [PATCH v5 11/39] x86/mm: Update pte_modify for _PAGE_COW

Edgecombe, Rick P
2023-02-10T17:00:40Z

On Fri, 2023-02-10 at 14:57 +0100, Borislav Petkov wrote:
> @@ -749,14 +740,8 @@ static inline pte_t pte_modify(pte_t pte,
> pgprot_t newprot)
>
> pte_result = __pte(val);
>
> - /*
> - * Dirty bit is not preserved above so it can be done
> - * in a special way for the shadow stack case, where it
> - * may need to set _PAGE_COW. __pte_mkdirty() will do this in
> - * the case of shadow stack.
> - */
> if (pte_dirty(pte))
> - pte_result = __pte_mkdirty(pte_result, false);
> + pte_result = pte_set_flags(pte_result, _PAGE_DIRTY);
>
> return pte_result;
> }
>

Oh, I see what you are seeing now. Did you notice that the
__pte_mkdirty() logic got expanded in "x86/mm: Start actually marking
_PAGE_COW"? So if we don't put that logic in a usable helper, it ends
up open coded with pte_modify() looking something like this:
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
pteval_t val = pte_val(pte), oldval = val;
pte_t pte_result;

/*
* Chop off the NX bit (if present), and add the NX portion of
* the newprot (if present):
*/
val &= (_PAGE_CHG_MASK & ~_PAGE_DIRTY);
val |= check_pgprot(newprot) & ~_PAGE_CHG_MASK;
val = flip_protnone_guard(oldval, val, PTE_PFN_MASK);

pte_result = __pte(val);

/*
* Dirty bit is not preserved above so it can be done
* in a special way for the shadow stack case, where it
* may need to set _PAGE_SAVED_DIRTY. __pte_mkdirty() will do
* this in the case of shadow stack.
*/
if (oldval & _PAGE_DIRTY)
if (cpu_feature_enabled(X86_FEATURE_USER_SHSTK) &&
!pte_write(pte_result))
pte_set_flags(pte_result, _PAGE_SAVED_DIRTY);
else
pte_set_flags(pte_result, _PAGE_DIRTY);
}

return pte_result;
}

So the later logic of doing the _PAGE_SAVED_DIRTY (_PAGE_COW) part is
not centralized. It's ok?