Re: [PATCH v3 1/6] 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 freed to the buddy allocator before their page table entries (PTEs) are cleared, creating a Use-After-Free and memory corruption window.
- [Medium] A sleeping memory allocation is performed inside a pte_fn_t callback under lazy_mmu_mode_enable(), violating core MM API contracts.
--

--- Patch [1]: [PATCH v3 1/6] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit 2a65969cbdc861d6caeb95bb64f45d4facecff0e
Author: Heiko Carstens <hca@linux.ibm.com>

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

The guest variable storage area is allocated with vmalloc and then
donated to the ultravisor. Any kernel access to that area will result
in a secure storage access exception (aka fault).

This is a problem if such a memory area is read via /proc/kcore. This
causes an exception via vread_iter() and results in an unexpected short
read. Avoid this by allocating a custom VM_SPARSE area.

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index a284f98d97168..d1cdc5ae5e270 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
[ ... ]
> @@ -209,6 +210,70 @@ int uv_convert_from_secure_pte(pte_t pte)
>  	return uv_convert_from_secure_folio(pfn_folio(pte_pfn(pte)));
>  }
>  
> +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]
Is it safe to free the physical page back to the buddy allocator while the
PTE remains fully present in the init_mm page tables?

The commit message notes that vfree() defers PTE updates, but vfree() actually
clears the PTEs via remove_vm_area() before freeing the underlying pages.
Here, the pages are freed first, and the PTEs are cleared later when
free_vm_area() is called in uv_free_stor_var().

Could this create a window where the physical page is reallocated to another
process, but the current CPU still has a valid, writable PTE pointing to it?

Any stray kernel access or speculative execution through the old vmalloc
address could then silently corrupt the new owner's memory without faulting.

> +	return 0;
> +}
> +
> +void uv_free_stor_var(void *stor_var)
> +{
[ ... ]
> +}
> +EXPORT_SYMBOL_FOR_MODULES(uv_free_stor_var, "kvm");
> +
> +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]
Does this violate the cross-architecture pte_fn_t API contract by performing
a sleeping memory allocation?

This callback is executed by apply_to_page_range(), which wraps the pte_fn_t
callbacks in a lazy MMU mode section. The MM subsystem guidelines dictate
that lazy MMU mode implies a possible atomic context (as it disables
preemption on some architectures), forbidding these callbacks from sleeping.

While lazy_mmu_mode_enable() is currently a no-op on s390, the commit message
states s390 expects to gain full support for lazy MMU mode in the future.
Will this GFP_KERNEL allocation then introduce a preemption-disabled deadlock?

> +	if (!page)
> +		return -ENOMEM;
[ ... ]

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