Re: [PATCH] mm: Fix the race between collapse and PT_RECLAIM under per-vma lock

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2025/8/5 16:17, Qi Zheng wrote:
> 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@oppo.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

__pte_offset_map() might not return NULL? Because the 'pmd_populate(mm, 
pmd, pmd_pgtable(_pmd))' could populate a valid page (although the 
'_pmd' entry is NONE), but it is not the original pagetable page.

> 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.
>>
>