| 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: <46f0b251-237c-421d-aec0-adff6c2e1bb4@linux.alibaba.com> Date: Tue, 5 Aug 2025 18:07:03 +0800 From: Baolin Wang <baolin.wang@...ux.alibaba.com> To: David Hildenbrand <david@...hat.com>, Qi Zheng <zhengqi.arch@...edance.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 2025/8/5 17:50, David Hildenbrand wrote: > 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? Yes. >> >> 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? :( Sorry for not being clear. Let me try again. In the race condition described above, the '_pmd' value is NONE, meaning that when restoring the pmd entry with ‘pmd_populate(mm, pmd, pmd_pgtable(_pmd))’, the 'pmd_pgtable(_pmd)' can return a struct page corresponding to pfn == 0 (cause the '_pmd' is NONE) to populate the pmd entry. Clearly, this pfn == 0 page is not a pagetable page, meaning the corresponding ptl lock of this page is not initialized. Additionally, from the boot dmesg, I can see that the BIOS reports an address range with pfn == 0, indicating that there is a struct page initialized for pfn == 0 (possibly a reserved page): [ 0.000000] BIOS-provided physical RAM map: [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable [ 0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved [ 0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved [ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000007ffdffff] usable [ 0.000000] BIOS-e820: [mem 0x000000007ffe0000-0x000000007fffffff] reserved [ 0.000000] BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved [ 0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved Of course, this is my theoretical analysis from the code perspective. If there are other race conditions, I would be very surprised:) > Wasn't the original issue all about a NULL-pointer de-reference while > *locking*? Yes. > 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 >
Powered by blists - more mailing lists