linux-kernel - Re: [PATCH] mm/mremap: Fix move_normal_pmd/retract_page

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b989a811-f764-4b3d-b536-be4e68c0638e@bytedance.com>
Date: Tue, 8 Oct 2024 15:58:00 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: David Hildenbrand <david@...hat.com>
Cc: Jann Horn <jannh@...gle.com>, akpm@...ux-foundation.org,
 linux-mm@...ck.org, willy@...radead.org, hughd@...gle.com,
 lorenzo.stoakes@...cle.com, joel@...lfernandes.org,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] mm/mremap: Fix move_normal_pmd/retract_page_tables race



On 2024/10/8 15:52, David Hildenbrand wrote:
> On 08.10.24 05:53, Qi Zheng wrote:
>> Hi Jann,
>>
>> On 2024/10/8 05:42, Jann Horn wrote:
>>
>> [...]
>>
>>>
>>> diff --git a/mm/mremap.c b/mm/mremap.c
>>> index 24712f8dbb6b..dda09e957a5d 100644
>>> --- a/mm/mremap.c
>>> +++ b/mm/mremap.c
>>> @@ -238,6 +238,7 @@ static bool move_normal_pmd(struct vm_area_struct 
>>> *vma, unsigned long old_addr,
>>>    {
>>>        spinlock_t *old_ptl, *new_ptl;
>>>        struct mm_struct *mm = vma->vm_mm;
>>> +    bool res = false;
>>>        pmd_t pmd;
>>>        if (!arch_supports_page_table_move())
>>> @@ -277,19 +278,25 @@ static bool move_normal_pmd(struct 
>>> vm_area_struct *vma, unsigned long old_addr,
>>>        if (new_ptl != old_ptl)
>>>            spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
>>> -    /* Clear the pmd */
>>>        pmd = *old_pmd;
>>> +
>>> +    /* Racing with collapse? */
>>> +    if (unlikely(!pmd_present(pmd) || pmd_leaf(pmd)))
>>
>> Since we already hold the exclusive mmap lock, after a racing
>> with collapse occurs, the pmd entry cannot be refilled with
>> new content by page fault. So maybe we only need to recheck
>> pmd_none(pmd) here?
> 
> My thinking was that it is cheap and more future proof to check that we 
> really still have a page table here. For example, what if collapse code 
> is ever changed to replace the page table by the collapsed PMD?

Ah, make sense.

Acked-by: Qi Zheng <zhengqi.arch@...edance.com>

> 
> So unless there is a good reason not to have this check here, I would 
> keep it like that.
> 
> Thanks!
>