| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7e5f27b5-392e-43bd-8a47-b81395dbf9a7@kernel.org>
Date: Mon, 9 Feb 2026 11:58:20 +0100
From: "David Hildenbrand (Arm)" <david@...nel.org>
To: Barry Song <21cnbao@...il.com>
Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>, Liam.Howlett@...cle.com,
akpm@...ux-foundation.org, catalin.marinas@....com, dev.jain@....com,
harry.yoo@...cle.com, jannh@...gle.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, lorenzo.stoakes@...cle.com, mhocko@...e.com,
riel@...riel.com, rppt@...nel.org, ryan.roberts@....com, surenb@...gle.com,
vbabka@...e.cz, will@...nel.org, willy@...radead.org
Subject: Re: [PATCH] mm: rmap: skip batched unmapping for UFFD vmas
On 2/9/26 11:49, Barry Song wrote:
> On Mon, Feb 9, 2026 at 5:54 PM David Hildenbrand (Arm) <david@...nel.org> wrote:
>>
>> On 1/16/26 17:26, Baolin Wang wrote:
>>> As Dev reported[1], it's not ready to support batched unmapping for uffd case.
>>> Let's still fallback to per-page unmapping for the uffd case.
>>>
>>> [1] https://lore.kernel.org/linux-mm/20260116082721.275178-1-dev.jain@arm.com/
>>> Reported-by: Dev Jain <dev.jain@....com>
>>> Suggested-by: Barry Song <baohua@...nel.org>
>>> Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>>> ---
>>> mm/rmap.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index f13480cb9f2e..172643092dcf 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -1953,6 +1953,9 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
>>> if (pte_unused(pte))
>>> return 1;
>>>
>>> + if (userfaultfd_wp(vma))
>>> + return 1;
>>> +
>>
>> Interesting. I was just wondering why we didn't run into that with lazyfree folios.
>>
>> Staring at pte_install_uffd_wp_if_needed(), we never set the marker for
>> anonymous VMAs.
>>
>> So, yeah, if one sets lazyfree on a uffd-wp PTE, the uffd-wp bit will just get
>> zapped alongside. Just like MADV_DONTNEED.
>>
>>
>> I'm fine with that temporary fix. But I guess the non-hacky way to handle this would be:
>>
>>
>> From 53d016d6e6f624425dbdbc2fb1dec7c91fbef15c Mon Sep 17 00:00:00 2001
>> From: "David Hildenbrand (Arm)" <david@...nel.org>
>> Date: Mon, 9 Feb 2026 10:52:59 +0100
>> Subject: [PATCH] tmp
>>
>> Signed-off-by: David Hildenbrand (Arm) <david@...nel.org>
>> ---
>> include/linux/mm_inline.h | 15 ++++++---------
>> mm/memory.c | 21 +--------------------
>> mm/rmap.c | 2 +-
>> 3 files changed, 8 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
>> index fa2d6ba811b5..8a9a2c5f5ee3 100644
>> --- a/include/linux/mm_inline.h
>> +++ b/include/linux/mm_inline.h
>> @@ -566,9 +566,8 @@ static inline pte_marker copy_pte_marker(
>> *
>> * Returns true if an uffd-wp pte was installed, false otherwise.
>> */
>> -static inline bool
>> -pte_install_uffd_wp_if_needed(struct vm_area_struct *vma, unsigned long addr,
>> - pte_t *pte, pte_t pteval)
>> +static inline bool install_uffd_wp_ptes_if_needed(struct vm_area_struct *vma,
>> + unsigned long addr, pte_t *pte, unsigned int nr, pte_t pteval)
>> {
>> bool arm_uffd_pte = false;
>>
>> @@ -598,13 +597,11 @@ pte_install_uffd_wp_if_needed(struct vm_area_struct *vma, unsigned long addr,
>> if (unlikely(pte_swp_uffd_wp_any(pteval)))
>> arm_uffd_pte = true;
>>
>> - if (unlikely(arm_uffd_pte)) {
>> - set_pte_at(vma->vm_mm, addr, pte,
>> - make_pte_marker(PTE_MARKER_UFFD_WP));
>> - return true;
>> - }
>> + if (likely(!arm_uffd_pte))
>> + return false;
>>
>> - return false;
>> + set_ptes(vma->vm_mm, addr, pte, make_pte_marker(PTE_MARKER_UFFD_WP), nr);
>> + return true;
>> }
>>
>> static inline bool vma_has_recency(const struct vm_area_struct *vma)
>> diff --git a/mm/memory.c b/mm/memory.c
>> index da360a6eb8a4..0a87d02a9a69 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -1592,29 +1592,10 @@ zap_install_uffd_wp_if_needed(struct vm_area_struct *vma,
>> unsigned long addr, pte_t *pte, int nr,
>> struct zap_details *details, pte_t pteval)
>> {
>> - bool was_installed = false;
>> -
>> - if (!uffd_supports_wp_marker())
>> - return false;
>> -
>> - /* Zap on anonymous always means dropping everything */
>> - if (vma_is_anonymous(vma))
>> - return false;
>> -
>> if (zap_drop_markers(details))
>> return false;
>>
>> - for (;;) {
>> - /* the PFN in the PTE is irrelevant. */
>> - if (pte_install_uffd_wp_if_needed(vma, addr, pte, pteval))
>> - was_installed = true;
>> - if (--nr == 0)
>> - break;
>> - pte++;
>> - addr += PAGE_SIZE;
>> - }
>> -
>> - return was_installed;
>> + return install_uffd_wp_ptes_if_needed(vma, addr, pte, nr, pteval);
>> }
>>
>> static __always_inline void zap_present_folio_ptes(struct mmu_gather *tlb,
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 7b9879ef442d..f71aacf35925 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -2061,7 +2061,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>> * we may want to replace a none pte with a marker pte if
>> * it's file-backed, so we don't lose the tracking info.
>> */
>> - pte_install_uffd_wp_if_needed(vma, address, pvmw.pte, pteval);
>> + install_uffd_wp_ptes_if_needed(vma, address, pvmw.pte, nr_pages, pteval);
>>
>> /* Update high watermark before we lower rss */
>> update_hiwater_rss(mm);
>> --
>> 2.43.0
>>
>>
>>
>> Does somebody have time to look into that? We should also adjust the doc of pte_install_uffd_wp_if_needed()
>> and turn it into some proper kerneldoc.
>
> I'd nominate Dev, if he has the time, as he has been working on
> related changes and is already familiar with this area :-)
>
> https://lore.kernel.org/linux-mm/20260116082721.275178-1-dev.jain@arm.com/
>
> I assume this could be treated as a separate optimization, as
> the current temporary fix seems acceptable?
Let's call it a cleanup, because the way try_to_unmap_one() sometimes
respects "nr_pages" and sometimes doesn't (because it's hidden somewhere
that nr_pages will be 1) is just nasty and provoked the bug you ran into
in the first place. :)
--
Cheers,
David
Powered by blists - more mailing lists