Re: [PATCH 6/9] x86/mm: Introduce ptep_set_wrprotect_flush and related functions

[Dave Hansen]

On 06/07/2018 09:24 AM, Andy Lutomirski wrote:
>> +static inline void ptep_set_wrprotect_flush(struct vm_area_struct *vma,
>> +                                           unsigned long addr, pte_t *ptep)
>> +{
>> +       bool rw;
>> +
>> +       rw = test_and_clear_bit(_PAGE_BIT_RW, (unsigned long *)&ptep->pte);
>> +       if (IS_ENABLED(CONFIG_X86_INTEL_SHADOW_STACK_USER)) {
>> +               struct mm_struct *mm = vma->vm_mm;
>> +               pte_t pte;
>> +
>> +               if (rw && (atomic_read(&mm->mm_users) > 1))
>> +                       pte = ptep_clear_flush(vma, addr, ptep);
> Why are you clearing the pte?

I think I insisted on this being in there.

First of all, we need to flush the TLB eventually because we need the
shadowstack PTE permissions to be in effect.

But, generally, we can't clear a dirty bit in a "live" PTE without
flushing.  The processor can keep writing until we flush, and even keep
setting it whenever _it_ allows a write, which it can do based on stale
TLB contents.  Practically, I think a walk to set the dirty bit is
mostly the same as a TLB miss, but that's certainly not guaranteed forever.

That's even ignoring all the fun errata we have.