Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dropping FOLL_LONGTERM creates a silent long-term pin, bypassing GUP safety checks for file-backed mappings and breaking CMA isolation for anonymous RAM.

Pre-existing issues:
- [Critical] Data race and unprotected state modification on `mirror->enc_context_owner` during SEV VM migration leads to list corruption and Use-After-Free.
--

commit a611d4c7ad0ddfba6a16536a997563da04d1672f
Author: Pankaj Gupta <pankaj.gupta@amd.com>

KVM: SEV: drop FOLL_LONGTERM for encrypted region registration

This commit removes the FOLL_LONGTERM flag from sev_pin_memory() when
registering encrypted memory regions. This aims to restore virtio-pmem
compatibility by allowing GUP to succeed on file-backed host mappings.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74fb15551e83f..053b59068b580 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -2751,7 +2751,7 @@ int sev_mem_enc_register_region(struct kvm *kvm,
>  		return -ENOMEM;
>  
>  	region->pages = sev_pin_memory(kvm, range->addr, range->size, &region->npages,
> -				       FOLL_WRITE | FOLL_LONGTERM);
> +				       FOLL_WRITE);

[Severity: High]
Does removing FOLL_LONGTERM here create a silent long-term pin?

By dropping FOLL_LONGTERM, it appears we bypass GUP safety checks for
file-backed mappings and break CMA isolation for anonymous RAM. This could
allow guest VMs to permanently pin movable pages, leading to CMA allocation
failures and breaking memory hot-unplug.

For file-backed mappings, could this cause filesystem operations like
truncate() to loop indefinitely waiting for page refcounts to drop?


[Severity: Critical]
This is a pre-existing issue, but is there a data race on
mirror->enc_context_owner during SEV VM migration?

Looking at sev_migrate_from(), it updates mirror->enc_context_owner without
synchronization with concurrent readers. If a mirror VM is destroyed via
sev_vm_destroy() concurrently with its owner being migrated, it can read
the old owner_kvm pointer locklessly:

arch/x86/kvm/svm/sev.c:sev_vm_destroy() {
...
		struct kvm *owner_kvm = sev->enc_context_owner;

		mutex_lock(&owner_kvm->lock);
		list_del(&sev->mirror_entry);
		mutex_unlock(&owner_kvm->lock);
		kvm_put_kvm(owner_kvm);
...
}

Could this block on the old source KVM lock while the migration thread moves
the mirror to the new destination KVM list?

When the migration thread releases the lock, sev_vm_destroy() would wake up,
attempt to delete the mirror from the list (corrupting the destination KVM
list), and incorrectly drop a second reference to the source KVM, causing a
use-after-free regression.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701144543.39582-1-pankaj.gupta@amd.com?part=1