Messages in this thread | | | Date | Fri, 27 Jan 2023 17:19:59 +0100 | From | David Hildenbrand <> | Subject | Re: [PATCH v5 18/39] mm: Handle faultless write upgrades for shstk |
| |
> > Now shadow stack memory creation is tightly controlled. Either created > via special syscall or automatically with a new thread.
Good, it would be valuable to document that somewhere ("Neve rapplies to VM_SHARED|VM_MAYSHARE VMAs").
[...]
>> >> The other thing I had in mind was that we have to make sure that >> we're >> not accidentally setting "Write=0,Dirty=1" in mk_pte() / >> pte_modify(). >> >> Assume we had a "Write=1,Dirty=1" PTE, and we effectively wrprotect >> using pte_modify(), we have to make sure to move the dirty bit to >> the >> saved_dirty bit. > > For the mk_pte() case, I don't think a Write=0,Dirty=1 prot could come > from anywhere. I guess the MAP_SHARED case is a little less bounded. We > could maybe add a warning for this case.
Right, Write=0,Dirty=1 shouldn't apply at that point if shstk are always wrprotected as default.
> > For the pte_modify() case, this does happen. There are two scenarios > considered: > 1. A Write=0,Dirty=0 PTE is made dirty. This can't happen today as > Dirty is filtered via _PAGE_CHG_MASK. Basically pte_modify() doesn't > support it.
It should simply set the saved_dirty bit I guess. But I don't think pte_modify() is actually supposed to set PTEs dirty (primary goal is to change protection IIRC).
> 2. A Write=1,Dirty=1 PTE gets write protected. This does happen because > the Write=0 prot comes from protection_map, and pte_modify() would > leave the Dirty=1 bit alone. The main case I know of is mprotect(). It > is handled by changes to pte_modify() by doing the Dirty->SoftDirty > fixup if needed.
Right, we'd have to move the dirty bit to the saved_dirty bit. (we have to handle soft-dirty, too, whenever setting the PTE dirty -- either via the dirty bit or via the saved_dirty bit)
> > So pte_modify()s job should not be too tricky. What you can't do with > it though, is create shadow stack PTEs. But it is ok for our uses > because of the explicit mkwrite().
I think you are correct.
-- Thanks,
David / dhildenb
| |