Re: [PATCH] mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()

[sashiko-bot@kernel.org]

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 <khorenko@virtuozzo.com>

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-d225c60bc316@virtuozzo.com?part=1