Messages in this thread | | | Date | Tue, 24 Jan 2023 17:28:22 +0100 | Subject | Re: [PATCH v5 10/39] x86/mm: Introduce _PAGE_COW | From | David Hildenbrand <> |
| |
On 23.01.23 21:56, Edgecombe, Rick P wrote: > 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?
It goes into the direction of my other proposal/idea, yes. Not sure if _PAGE_SAVED_DIRTY would currently mimic what's happening here ... _PAGE_COW is certainly wrong and misleading.
-- Thanks,
David / dhildenb
| |