On 4/7/20 11:14 AM, Yu-cheng Yu wrote:
> On Wed, 2020-02-26 at 16:08 -0800, Dave Hansen wrote:
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 45442d9a4f52..6daa28614327 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -772,7 +772,8 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>>> * If it's a COW mapping, write protect it both
>>> * in the parent and the child
>>> */
>>> - if (is_cow_mapping(vm_flags) && pte_write(pte)) {
>>> + if ((is_cow_mapping(vm_flags) && pte_write(pte)) ||
>>> + arch_copy_pte_mapping(vm_flags)) {
>>> ptep_set_wrprotect(src_mm, addr, src_pte);
>>> pte = pte_wrprotect(pte);
>>> }
>>
>> You have to modify this because pte_write()==0 for shadow stack PTEs, right?
>>
>> Aren't shadow stack ptes *logically* writable, even if they don't have
>> the write bit set? What would happen if we made pte_write()==1 for them?
>
> Here the vm_flags needs to have VM_MAYWRITE, and the PTE needs to have
> _PAGE_WRITE. A shadow stack does not have either.
I literally mean taking pte_write(), and doing something l
static inline int pte_write(pte_t pte)
{
if (pte_present(pte) && pte_is_shadow_stack(pte))
return 1;
return pte_flags(pte) & _PAGE_RW;
}
Then if is_cow_mapping() returns true for shadow stack VMAs, the above
code doesn't need to change.
> To fix checking vm_flags, what about adding a "arch_is_cow_mappping()" to the
> generic is_cow_mapping()?
That makes good sense to me.
> For the PTE, the check actually tries to determine if the PTE is not already
> being copy-on-write, which is:
>
> (!_PAGE_RW && !_PAGE_DIRTY_HW)
>
> So what about making it pte_cow()?
>
> /*
> * The PTE is in copy-on-write status.
> */
> static inline int pte_cow(pte_t pte)
> {
> return !(pte_flags(pte) & (_PAGE_WRITE | _PAGE_DIRTY_HW));
> }
... with appropriate comments that seems fine to me.
