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

[Heiko Carstens <hca@linux.ibm.com>]

On Fri, Jul 17, 2026 at 11:12:09AM +0200, Christian Borntraeger wrote:
> Am 16.07.26 um 16:33 schrieb sashiko-bot@kernel.org:
> [...]
> > [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?
> 
> I guess this is a very very theoretical problem. Not sure if we really want
> to address this. The necessary allocation in a freeing function might be
> worse than temporary access via the kernel mapping.

I added a comment why this won't be addressed. The existing vmalloc
code already implements lazy TLB flushing (even though it is not
active on only s390 because of our special ptep_get_and_clear()
implementation).

> > > +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:
> 
> This is one of those things that wont happen in reality as the returned sized
> will be aligned, but as the interface for base variable storage returns bytes
> and in theory _COULD_ be unaligned
> maybe change belows hunk to
> 
> @@ -369,7 +369,7 @@ static int kvm_s390_pv_alloc_vm(struct kvm *kvm)
>  	/* Allocate variable storage */
>  	vlen = ALIGN(virt * ((npages * PAGE_SIZE) / HPAGE_SIZE), PAGE_SIZE);
> ---->	vlen += uv_info.guest_virt_base_stor_len;   <--- also add ALIGN here
> -	kvm->arch.pv.stor_var = vzalloc(vlen);
> +	kvm->arch.pv.stor_var = uv_alloc_stor_var(vlen);
>  	if (!kvm->arch.pv.stor_var)
>  		goto out_err;
>  	return 0;
> 
> Could also be a separate patch if we want to.

I added the alignment handling uv_alloc_stor_var(), so that it doesn't
matter what callers pass.