Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mm: NULL pointer dereference in zap_huge_pmd() for special PMDs on PowerPC --
commit 5100f57130de5473973d1370273238b4778fae24 Author: Konstantin Khorenko <[email protected]> mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd() copy_huge_pmd() reads the source PMD without the page table lock and, for a special PMD, installs that value in the child without reading it again under the lock. This commit reads the PMD again under the lock to ensure it wasn't cleared concurrently by unmap_mapping_range(). > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 1e5d68acf62a5..fcea98f0d8865 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -1965,13 +1965,22 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct > mm_struct *src_mm, > src_ptl = pmd_lockptr(src_mm, src_pmd); > spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING); > /* > - * No need to recheck the pmd, it can't change with write > - * mmap lock held here. > - * > - * Meanwhile, making sure it's not a CoW VMA with writable > - * mapping, otherwise it means either the anon page wrongly > - * applied special bit, or we made the PRIVATE mapping be > - * able to wrongly write to the backend MMIO. > + * The mmap and VMA write locks of the source keep faults > + * out, but unmap_mapping_range() does not take the mmap > + * lock and may have cleared the pmd since the lockless > + * read. Let copy_pmd_range() skip it then. > + */ > + pmd = pmdp_get(src_pmd); > + if (unlikely(!pmd_present(pmd) || !pmd_special(pmd))) { > + spin_unlock(src_ptl); > + spin_unlock(dst_ptl); > + return -EAGAIN; > + } > + /* > + * Make sure it's not a CoW VMA with writable mapping, > + * otherwise it means either the anon page wrongly applied > + * special bit, or we made the PRIVATE mapping be able to > + * wrongly write to the backend MMIO. > */ > VM_WARN_ON_ONCE(vma_is_cow_mapping(src_vma) && pmd_write(pmd)); > goto set_pmd; [Severity: High] Does this code cause a NULL pointer dereference on PowerPC when a special PMD is unmapped by the child? Because the jump to set_pmd bypasses pgtable allocation and deposition for the child process's new special PMD, no pgtable is deposited. Later, when the child unmaps the VMA or exits, zap_pmd_range() calls zap_huge_pmd(). On architectures like PowerPC (hash mode) where arch_needs_pgtable_deposit() is true, this leads to: mm/huge_memory.c:zap_huge_pmd() { ... if (has_deposit) zap_deposited_table(mm, pmd); ... } This will unconditionally assume a deposited table exists and try to withdraw it. Then in zap_deposited_table(): mm/huge_memory.c:zap_deposited_table() { ... pgtable = pgtable_trans_huge_withdraw(mm, pmd); pte_free(mm, pgtable); ... } Since no page table was ever deposited, pgtable_trans_huge_withdraw() returns NULL, and passing NULL to pte_free() leads to a kernel crash. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v1-1-d225c60bc...@virtuozzo.com?part=1
