Re: [PATCH v2 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 kernel page table entries (PTEs) are cleared and TLBs are flushed, creating a dangerous Use-After-Free window.
- [High] The page table walker callback `uv_alloc_range_cb` performs a sleeping memory allocation (`GFP_KERNEL_ACCOUNT`) while holding a spinlock.
--

--- Patch [1]: [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit 3aa87ec52bc96aa3b06650c2d3924417b23be4ca
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 commit changes the allocation to use
a VM_SPARSE area so that if the area is read via /proc/kcore,
vread_iter() returns zeroes instead of causing an exception.

> 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]
Does this create a window where the physical page is freed to the buddy
allocator but still mapped in the kernel page tables?

If another CPU reallocates the freed page before free_vm_area() unmaps and
flushes the TLB, could speculative execution or a stray kernel access to the
old virtual address read or corrupt the new owner's data?

> +	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);
> +}
> +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: High]
Can this allocation sleep while holding a spinlock?

The apply_to_page_range() function loops over the page tables and acquires
the page table spinlock (ptl) via pte_alloc_map_lock() before invoking the
callback.

Since GFP_KERNEL_ACCOUNT implies __GFP_DIRECT_RECLAIM, which can sleep
and invoke the scheduler, could calling this while holding the page table
spinlock trigger a scheduling while atomic bug or deadlock?

> +	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/20260717092415.3039699-1-hca@linux.ibm.com?part=1