linux-kernel - Re: [PATCH] mm: Fix the race between collapse and PT

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <b594a315-8458-439c-b535-fc7f052a3fae@bytedance.com>
Date: Tue, 5 Aug 2025 16:17:01 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Barry Song <21cnbao@...il.com>, akpm@...ux-foundation.org, linux-mm@...ck.org
Cc: linux-kernel@...r.kernel.org, Barry Song <v-songbaohua@...o.com>,
 "Lai, Yi" <yi1.lai@...ux.intel.com>, David Hildenbrand <david@...hat.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
 Suren Baghdasaryan <surenb@...gle.com>, Lokesh Gidra
 <lokeshgidra@...gle.com>, Tangquan Zheng <zhengtangquan@...o.com>,
 Lance Yang <ioworker0@...il.com>, Zi Yan <ziy@...dia.com>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>, Nico Pache
 <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
 Dev Jain <dev.jain@....com>
Subject: Re: [PATCH] mm: Fix the race between collapse and PT_RECLAIM under
 per-vma lock

Hi Baolin,

On 8/5/25 3:53 PM, Baolin Wang wrote:
> 
> 
> On 2025/8/5 14:42, Qi Zheng wrote:
>> Hi Barry,
>>
>> On 8/5/25 11:54 AM, Barry Song wrote:
>>> From: Barry Song <v-songbaohua@...o.com>
>>>
>>> The check_pmd_still_valid() call during collapse is currently only
>>> protected by the mmap_lock in write mode, which was sufficient when
>>> pt_reclaim always ran under mmap_lock in read mode. However, since
>>> madvise_dontneed can now execute under a per-VMA lock, this assumption
>>> is no longer valid. As a result, a race condition can occur between
>>> collapse and PT_RECLAIM, potentially leading to a kernel panic.
>>
>> There is indeed a race condition here. And after applying this patch, I
>> can no longer reproduce the problem locally (I was able to reproduce it
>> stably locally last night).
>>
>> But I still can't figure out how this race condtion causes the
>> following panic:
>>
>> exit_mmap
>> --> mmap_read_lock()
>>      unmap_vmas()
>>      --> pte_offset_map_lock
>>          --> rcu_read_lock()
>>              check if the pmd entry is a PTE page
>>              ptl = pte_lockptr(mm, &pmdval)  <-- ptl is NULL
>>              spin_lock(ptl)                  <-- PANIC!!
>>
>> If this PTE page is freed by pt_reclaim (via RCU), then the ptl can 
>> not be NULL.
>>
>> The collapse holds mmap write lock, so it is impossible to be concurrent
>> with exit_mmap().
>>
>> Confusing. :(
> 
> IIUC, the issue is not caused by the concurrency between exit_mmap and 
> collapse, but rather by the concurrency between pt_reclaim and collapse.
> 
> Before this patch, khugepaged might incorrectly restore a PTE pagetable 
> that had already been freed.
> 
> pt_reclaim has cleared the pmd entry and freed the PTE page table. 
> However, due to the race condition, check_pmd_still_valid() still passes 
> and continues to attempt the collapse:
> 
> _pmd = pmdp_collapse_flush(vma, address, pmd); ---> returns a none pmd 
> entry (the original pmd entry has been cleared)
> 
> pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl); ---> returns 
> pte == NULL
> 
> Then khugepaged will restore the old PTE pagetable with an invalid pmd 
> entry:
> 
> pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> 
> So when the process exits and trys to free the mapping of the process, 
> traversing the invalid pmd table will lead to a crash.

CPU0                         CPU1
====                         ====

collapse
--> pmd_populate(mm, pmd, pmd_pgtable(_pmd));
     mmap_write_unlock
                              exit_mmap
                              --> hold mmap lock
                                  __pte_offset_map_lock
                                  --> pte = __pte_offset_map(pmd, addr, 
&pmdval);
                                      if (unlikely(!pte))
                                          return pte;   <-- will return

IIUC, in this case, if we get an invalid pmd entry, we will retrun
directly instead of causing a crash?

> 
> Barry, please correct me if I have misunderstood something.
>