[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5ac2ec58-3908-4d0e-a29b-8b4d776410e3@redhat.com>
Date: Tue, 5 Aug 2025 11:50:36 +0200
From: David Hildenbrand <david@...hat.com>
To: Qi Zheng <zhengqi.arch@...edance.com>,
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>,
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
On 05.08.25 11:30, Qi Zheng wrote:
>
>
> On 8/5/25 4:56 PM, Baolin Wang wrote:
>>
>>
>> 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@...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
>>
>> __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.
>
> CPU0 CPU1
> ==== ====
>
> collapse
> --> check_pmd_still_valid
> vma read lock
> pt_reclaim clear the pmd entry and will
> free the PTE page (via RCU)
> vma read unlock
>
> vma write lock
> _pmd = pmdp_collapse_flush(vma, address, pmd) <-- pmd_none(_pmd)
> pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl); <-- pte is
> NULL
> pmd_populate(mm, pmd, pmd_pgtable(_pmd)); <-- populate a valid page?
> vma write unlock
>
> The above is the concurrent scenario you mentioned, right?
>
> What types of this 'valid page' could be? If __pte_offset_map() returns
> non-NULL, then it is a PTE page. Even if it is not the original one, it
> should not cause panic. Did I miss some key information? :(
Wasn't the original issue all about a NULL-pointer de-reference while
*locking*?
Note that in that kernel config [1] we have CONFIG_DEBUG_SPINLOCK=y, so
likely we will have ALLOC_SPLIT_PTLOCKS set.
[1]
https://github.com/laifryiee/syzkaller_logs/blob/main/250803_193026___pte_offset_map_lock/.config
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists