Re: [PATCH v2] arm64/mm: Check the requested PFN range during memory removal

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The lockless page table walk in `addr_splits_kernel_leaf` dereferences shared PUD and PMD pointers without `READ_ONCE()`, violating ARM64 subsystem rules.
--

--- Patch [1]: [PATCH v2] arm64/mm: Check the requested PFN range during memory removal ---
commit bacbc9fb9b238d0e34546abe25a64655eea1704f
Author: Richard Cheng <[email protected]>
arm64/mm: Check the requested PFN range during memory removal

This commit fixes the memory removal notifier to check the correct
requested PFN range rather than the range following it. It does this
by passing arg->start_pfn into can_unmap_without_split() instead of
the already advanced pfn variable.

> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index a25d8beacc831..18a8b0d3714ee 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
[ ... ]
> @@ -2194,7 +2194,7 @@ static int prevent_memory_remove_notifier(struct notifier_block *nb,
>  		}
>  	}
>  
> -	if (!can_unmap_without_split(pfn, arg->nr_pages))
> +	if (!can_unmap_without_split(arg->start_pfn, arg->nr_pages))

[Severity: High]
This is a pre-existing issue, but does the page table walk in
addr_splits_kernel_leaf() (called by can_unmap_without_split() here) need
to use READ_ONCE() for its shared pointer dereferences?

Looking at addr_splits_kernel_leaf(), it performs a lockless walk of the
kernel page tables. It passes pudp to pmd_offset() and pmdp to
pte_offset_kernel():

arch/arm64/mm/mmu.c:addr_splits_kernel_leaf() {
        ...
        pmdp = pmd_offset(pudp, addr);
        pmd = pmdp_get(pmdp);
        ...
        ptep = pte_offset_kernel(pmdp, addr);
        pte = __ptep_get(ptep);
        ...
}

On arm64, the generic implementations of pmd_offset() and
pte_offset_kernel() perform plain loads instead of using READ_ONCE().

If there is a concurrent modification of the kernel page tables (such as a
split of a linear map huge page triggered by secretmem), could the compiler
observe a torn or stale pointer here?

Dereferencing this through pmdp_get() or __ptep_get() might result in
accessing a bogus physical address and causing a kernel panic.

>  		return NOTIFY_BAD;
>  
>  	return NOTIFY_OK;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1