Re: [PATCH v4 1/8] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Physical pages are returned to the buddy allocator while their page table entries (PTEs) are left valid, creating a Use-After-Free / Map-After-Free window.
- [Medium] Sleeping memory allocation within a `pte_fn_t` callback violates the `lazy_mmu_mode` API contract.
--

--- Patch [1]: [PATCH v4 1/8] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit 66226034e9e83eee2fe85a1e6e86baa666a0331b
Author: Heiko Carstens <hca@linux.ibm.com>

KVM: s390: pv: Use VM_SPARSE area for guest variable storage area

The patch replaces a standard vmalloc allocation for the guest variable
storage area with a custom VM_SPARSE area allocated via get_vm_area(). It
populates and tears down this memory using apply_to_page_range() and
apply_to_existing_page_range() callbacks.

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c

[ ... ]

> +static int uv_free_range_cb(pte_t *ptep, unsigned long addr, void *data)
> +{
> +	pte_t pte = ptep_get(ptep);
> +
> +	if (!pte_present(pte))
> +		return 0;
> +	/*
> +	 * Note: do not update the pte here, since there is no code which
> +	 * accesses the memory range, besides bugs. The invalidation of ptes
> +	 * and TLB flushing is deferred like for regular vfree() calls.
> +	 */
> +	__free_page(pte_page(pte));

[Severity: High]
Does freeing the physical page without clearing the page table entry introduce
a regression where the memory could be reallocated while the kernel retains
a valid mapping to it?

The comment mentions that vfree() defers PTE invalidation, but standard
vfree() clears PTEs synchronously and only defers the TLB flush.

Because apply_to_existing_page_range() returns before free_vm_area() is
called to unmap the area, could speculative execution or a hardware page
table walk access the new owner's memory after it has been returned to the
buddy allocator?

> +	return 0;
> +}

[ ... ]

> +static int uv_alloc_range_cb(pte_t *ptep, unsigned long addr, void *data)
> +{
> +	struct page *page;
> +	pte_t pte;
> +
> +	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);

[Severity: Medium]
Is it safe to use sleeping allocation flags during this page table walk?

The apply_to_page_range() function wraps iteration in lazy_mmu_mode_enable()
and lazy_mmu_mode_disable(). The MM page table guidelines state that lazy
MMU mode implies a possible atomic context because it disables preemption on
some architectures, meaning pte_fn_t callbacks must not sleep.

Could this cause a regression if the callback blocks, and would it be better
to use a non-sleeping allocation flag to conform to the apply_to_page_range()
contract?

> +	if (!page)
> +		return -ENOMEM;
> +	pte = __pte(page_to_phys(page) | pgprot_val(PAGE_KERNEL));
> +	set_pte(ptep, pte);
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085834.898025-1-hca@linux.ibm.com?part=1