lkml.org 
[lkml]   [2023]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [syzbot] [mm?] WARNING in unmap_page_range (2)
From
> 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

\
 
 \ /
  Last update: 2023-11-20 14:01    [W:0.038 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site