Re: [PATCH 1/4] 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 corresponding page table entries are cleared and the TLB is flushed, creating a Use-After-Free vulnerability window.
- [High] Passing an unaligned `size` to `uv_alloc_stor_var()` causes an infinite loop in the kernel page table walker.
--

--- Patch [1]: [PATCH 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit fd3cc6954f3085a22cae9f0932705a2913ea0050
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. This patch allocates it via a custom
VM_SPARSE area to avoid unexpected short reads via /proc/kcore.

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index a284f98d97168..98a7f988fc691 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c

[ ... ]

> @@ -209,6 +210,64 @@ 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;
> +	__free_page(pte_page(pte));
> +	return 0;
> +}
> +
> +void uv_free_stor_var(void *stor_var)
> +{
> +	unsigned long addr, size;
> +	struct vm_struct *area;
> +
> +	if (!stor_var)
> +		return;
> +	area = find_vm_area(stor_var);
> +	if (WARN_ON_ONCE(!area || !(area->flags & VM_SPARSE)))
> +		return;
> +	size = get_vm_area_size(area);
> +	addr = (unsigned long)area->addr;
> +	apply_to_existing_page_range(&init_mm, addr, size, uv_free_range_cb, NULL);
> +	free_vm_area(area);

[Severity: High]
Does this create a Use-After-Free window during VM teardown?

In uv_free_stor_var(), we call apply_to_existing_page_range() which invokes
uv_free_range_cb() for each page. The callback frees the physical page
directly to the buddy allocator via __free_page() while the PTE remains
fully present and writable in the init_mm page tables.

After all pages are freed, free_vm_area() is called to clear the PTEs and
flush the TLB. If a freed page is immediately reallocated to another
subsystem, can concurrent accesses via the stale vmalloc alias read or
overwrite the newly reallocated page before free_vm_area() completes?

> +}
> +EXPORT_SYMBOL_FOR_MODULES(uv_free_stor_var, "kvm");

[ ... ]

> +void *uv_alloc_stor_var(unsigned long size)
> +{
> +	struct vm_struct *area;
> +	unsigned long addr;
> +
> +	area = get_vm_area(size, VM_SPARSE);
> +	if (!area)
> +		return NULL;
> +	addr = (unsigned long)area->addr;
> +	if (apply_to_page_range(&init_mm, addr, size, uv_alloc_range_cb, NULL))

[Severity: High]
If an unaligned size is passed to uv_alloc_stor_var(), could this cause an
infinite loop in the page table walker?

The size parameter is passed directly to apply_to_page_range(). The internal
page table walker loop relies on the end boundary being exactly page-aligned
to terminate:

	do {
		...
	} while (pte++, addr += PAGE_SIZE, addr != end);

If size is not a multiple of PAGE_SIZE, addr will increment past end without
matching it, looping indefinitely over the page tables.

Should we pass the page-aligned size of the allocated VM area instead, perhaps
by using get_vm_area_size(area) or explicitly aligning size beforehand?

> +		goto out;
> +	return area->addr;
> +out:
> +	uv_free_stor_var(area->addr);
> +	return NULL;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(uv_alloc_stor_var, "kvm");

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