On Mon, Oct 21, 2024 at 03:45:31PM +0200, Vlastimil Babka wrote: > On 10/20/24 18:20, Lorenzo Stoakes wrote: > > Add a new PTE marker that results in any access causing the accessing > > process to segfault. > > Should we distinguish it from other segfaults? Is there a way? I can see > memory protection keys use SEGV_PKUERR, but no idea if we have any free > values.
Wasn't even aware that existed!! I'm not sure a process can do anything particularly useful with this information though? Hitting a guard page would indicate a programming error rather than something that might allow meaningful feedback to a user like memory protection keys. Do you think there's enough value int his to warrant digging in? And indeed I imagine bits are in short supply for this and would need a strong argument to get... so yeah I don't think too worthwhile most likely! Thanks for the suggestion though! > > > This is preferable to PTE_MARKER_POISONED, which results in the same > > handling as hardware poisoned memory, and is thus undesirable for cases > > where we simply wish to 'soft' poison a range. > > > > This is in preparation for implementing the ability to specify guard pages > > at the page table level, i.e. ranges that, when accessed, should cause > > process termination. > > > > Additionally, rename zap_drop_file_uffd_wp() to zap_drop_markers() - the > > function checks the ZAP_FLAG_DROP_MARKER flag so naming it for this single > > purpose was simply incorrect. > > > > We then reuse the same logic to determine whether a zap should clear a > > guard entry - this should only be performed on teardown and never on > > MADV_DONTNEED or the like. > > > > Signed-off-by: Lorenzo Stoakes <[email protected]> > > Acked-by: Vlastimil Babka <[email protected]> > > A nit below: > > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index 906294ac85dc..50e3f6ed73ac 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -6353,6 +6353,9 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct > > vm_area_struct *vma, > > ret = VM_FAULT_HWPOISON_LARGE | > > VM_FAULT_SET_HINDEX(hstate_index(h)); > > goto out_mutex; > > + } else if (marker & PTE_MARKER_GUARD) { > > + ret = VM_FAULT_SIGSEGV; > > + goto out_mutex; > > Given we don't support hugetlb, should we WARN_ON_ONCE() if such unexpected > marker appears there? > > > } > > } > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 0f614523b9f4..551455cd453f 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -1455,7 +1455,7 @@ static inline bool should_zap_folio(struct > > zap_details *details, > > return !folio_test_anon(folio); > > } > > > > -static inline bool zap_drop_file_uffd_wp(struct zap_details *details) > > +static inline bool zap_drop_markers(struct zap_details *details) > > { > > if (!details) > > return false; > > @@ -1476,7 +1476,7 @@ zap_install_uffd_wp_if_needed(struct vm_area_struct > > *vma, > > if (vma_is_anonymous(vma)) > > return; > > > > - if (zap_drop_file_uffd_wp(details)) > > + if (zap_drop_markers(details)) > > return; > > > > for (;;) { > > @@ -1671,7 +1671,15 @@ static unsigned long zap_pte_range(struct mmu_gather > > *tlb, > > * drop the marker if explicitly requested. > > */ > > if (!vma_is_anonymous(vma) && > > - !zap_drop_file_uffd_wp(details)) > > + !zap_drop_markers(details)) > > + continue; > > + } else if (is_guard_swp_entry(entry)) { > > + /* > > + * Ordinary zapping should not remove guard PTE > > + * markers. Only do so if we should remove PTE markers > > + * in general. > > + */ > > + if (!zap_drop_markers(details)) > > continue; > > } else if (is_hwpoison_entry(entry) || > > is_poisoned_swp_entry(entry)) { > > @@ -4003,6 +4011,10 @@ static vm_fault_t handle_pte_marker(struct vm_fault > > *vmf) > > if (marker & PTE_MARKER_POISONED) > > return VM_FAULT_HWPOISON; > > > > + /* Hitting a guard page is always a fatal condition. */ > > + if (marker & PTE_MARKER_GUARD) > > + return VM_FAULT_SIGSEGV; > > + > > if (pte_marker_entry_uffd_wp(entry)) > > return pte_marker_handle_uffd_wp(vmf); > > >
