Re: [PATCH v5 10/39] x86/mm: Introduce _PAGE_COW

[Edgecombe, Rick P]

Trying to answer both questions to this patch on this one.

On Mon, 2023-01-23 at 10:28 +0100, David Hildenbrand wrote:
> > +/*
> > + * Normally COW memory can result in Dirty=1,Write=0 PTEs. But in
> > the case
> > + * of X86_FEATURE_USER_SHSTK, the software COW bit is used, since
> > the
> > + * Dirty=1,Write=0 will result in the memory being treated as
> > shadow stack
> > + * by the HW. So when creating COW memory, a software bit is used
> > + * _PAGE_BIT_COW. The following functions pte_mkcow() and
> > pte_clear_cow()
> > + * take a PTE marked conventionally COW (Dirty=1) and transition
> > it to the
> > + * shadow stack compatible version of COW (Cow=1).
> > + */
> 
> TBH, I find that all highly confusing.
> 
> Dirty=1,Write=0 does not indicate a COW page reliably. You could
> have 
> both, false negatives and false positives.
> 
> False negative: fork() on a clean anon page.
> 
> False positives: wrpotect() of a dirty anon page.
> 
> 
> I wonder if it really has to be that complicated: what you really
> want 
> to achieve is to disallow "Dirty=1,Write=0" if it's not a shadow
> stack 
> page, correct?

The other thing is to save that the PTE is/was Dirty=1 somewhere (for
non-shadow stack memory). A slightly different but related thing. But
losing that information would would introduce differences for
pte_dirty() between when shadow stack was enabled or not. GUP/COW
doesn't need this anymore but there are lots of other places it gets
checked.

Perhaps following your GUP changes, _PAGE_COW is just now the wrong
name for it. _PAGE_SAVED_DIRTY maybe?