Messages in this thread | | | Date | Thu, 16 Nov 2023 19:13:44 +0100 | Subject | Re: [syzbot] [mm?] WARNING in unmap_page_range (2) | From | David Hildenbrand <> |
| |
> It should be fine, as: > > static void make_uffd_wp_pte(struct vm_area_struct *vma, > unsigned long addr, pte_t *pte) > { > pte_t ptent = ptep_get(pte); > > #ifndef CONFIG_USERFAULTFD_ > > if (pte_present(ptent)) { > pte_t old_pte; > > old_pte = ptep_modify_prot_start(vma, addr, pte); > ptent = pte_mkuffd_wp(ptent); > ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent); > } else if (is_swap_pte(ptent)) { > ptent = pte_swp_mkuffd_wp(ptent); > set_pte_at(vma->vm_mm, addr, pte, ptent); > } else { <----------------- this must be pte_none() already > set_pte_at(vma->vm_mm, addr, pte, > make_pte_marker(PTE_MARKER_UFFD_WP)); > } > }
Indeed! Is pte_swp_mkuffd_wp() reasonable for pte markers? I rememebr that we don't support multiple markers yet, so it might be good enough.
> >> >> 2) We get the error on arm64, which does *not* support uffd-wp. Do we >> maybe end up calling make_uffd_wp_pte() and place a pte marker, even >> though we don't have CONFIG_PTE_MARKER_UFFD_WP? >> >> >> static inline bool pte_marker_entry_uffd_wp(swp_entry_t entry) >> { >> #ifdef CONFIG_PTE_MARKER_UFFD_WP >> return is_pte_marker_entry(entry) && >> (pte_marker_get(entry) & PTE_MARKER_UFFD_WP); >> #else >> return false; >> #endif >> } >> >> Will always return false without CONFIG_PTE_MARKER_UFFD_WP. >> >> But make_uffd_wp_pte() might just happily place an entry. Hm. >> >> >> The following might fix the problem: >>
[...]
> > I'd like to double check with Muhammad (as I didn't actually follow his > work in the latest versions.. quite a lot changed), but I _think_ > fundamentally we missed something important in the fast path, and I think > it applies even to archs that support uffd.. > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index e91085d79926..3b81baabd22a 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -2171,7 +2171,8 @@ static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, > return 0; > } > > - if (!p->vec_out) { > + if (!p->vec_out && > + (p->arg.flags & PM_SCAN_WP_MATCHING))
Ouch, yes. So that's the global fence I was wondering where to find it.
-- Cheers,
David / dhildenb
| |