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

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

Pre-existing issues:
- [High] `addr_splits_kernel_leaf()` violates ARM64 lockless page table walk guidelines by performing naked dereferences and failing to account for folded topologies.
--

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

arm64/mm: Check the requested PFN range during memoroy removal

prevent_memory_remove_notifier() advances pfn while checking whether the
range contains early memory, which causes can_unmap_without_split() to check
the wrong range. This patch passes arg->start_pfn instead to ensure the
leaf-split check examines the requested range.

> 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 addr_splits_kernel_leaf() (called by
can_unmap_without_split()) violate lockless page table walk guidelines?

In arch/arm64/mm/mmu.c:addr_splits_kernel_leaf():

	p4dp = p4d_offset(pgdp, addr);
	p4d = p4dp_get(p4dp);

When P4D is folded, p4d_offset() returns pgdp. p4dp_get() then issues a
second READ_ONCE(*pgdp), which can observe a transiently cleared state if
the entry is modified concurrently (e.g. during a Break-Before-Make
sequence).

Additionally, lower down in the same function:

	ptep = pte_offset_kernel(pmdp, addr);
	pte = __ptep_get(ptep);

Is it possible for pte_offset_kernel() to directly dereference the shared
pmdp pointer without READ_ONCE(), violating lockless walk atomicity
requirements?

If the lockless walk observes a transiently cleared page table entry, it
could incorrectly return false, falsely indicating that the memory range
does not split a leaf mapping. This would allow a memory hot-remove
operation to proceed and unmap an entire leaf, unmapping adjacent live
kernel memory and causing a synchronous data abort on the next kernel
access.

>  		return NOTIFY_BAD;
>  
>  	return NOTIFY_OK;

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