Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
@ 2026-07-01 14:45 Pankaj Gupta
  2026-07-01 14:58 ` sashiko-bot
  2026-07-01 16:25 ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 6+ messages in thread
From: Pankaj Gupta @ 2026-07-01 14:45 UTC (permalink / raw)
  To: seanjc, pbonzini, tglx, mingo, dave.hansen
  Cc: bp, x86, thomas.lendacky, hpa, david, yangge1116, kvm,
	linux-kernel, pankaj.gupta, stable

commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
FOLL_WRITE | FOLL_LONGTERM since:

commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").

Drop FOLL_LONGTERM when registering encrypted memory regions and restore
the previous behavior.

Fixes: 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
Cc: stable@vger.kernel.org

Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
---
 arch/x86/kvm/svm/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6c6a6d663e29..c4b53700f69e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2764,7 +2764,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);
 	if (IS_ERR(region->pages)) {
 		ret = PTR_ERR(region->pages);
 		goto e_free;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
  2026-07-01 14:45 [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration Pankaj Gupta
@ 2026-07-01 14:58 ` sashiko-bot
  2026-07-01 16:25 ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-01 14:58 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: kvm

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
  2026-07-01 14:45 [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration Pankaj Gupta
  2026-07-01 14:58 ` sashiko-bot
@ 2026-07-01 16:25 ` David Hildenbrand (Arm)
  2026-07-01 16:30   ` Sean Christopherson
  1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-01 16:25 UTC (permalink / raw)
  To: Pankaj Gupta, seanjc, pbonzini, tglx, mingo, dave.hansen
  Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
	stable

On 7/1/26 16:45, Pankaj Gupta wrote:
> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> FOLL_WRITE | FOLL_LONGTERM since:
> 
> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
> 
> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> the previous behavior.

But that breaks the original issue of breaking ZONE_MOVABLE/CMA?

If it is a longterm pin, it must use FOLL_LONGTERM. :/

I assume we fail in check_vma_flags()

	if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
		return -EOPNOTSUPP;

IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?

How does vfio deal with that? (does it?)

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
  2026-07-01 16:25 ` David Hildenbrand (Arm)
@ 2026-07-01 16:30   ` Sean Christopherson
  2026-07-01 16:39     ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2026-07-01 16:30 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
	thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable

On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> On 7/1/26 16:45, Pankaj Gupta wrote:
> > commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> > added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> > migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> > virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> > FOLL_WRITE | FOLL_LONGTERM since:
> > 
> > commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> > commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
> > 
> > Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> > the previous behavior.
> 
> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?

Ya.

> If it is a longterm pin, it must use FOLL_LONGTERM. :/

Heh, well, KVM showed that that's not entirely true for many years :-)

Assuming we can't solve this some other way, and that there are "real" use cases
that were broken by adding FOLL_LONGTERM, maybe this as a hack-a-fix?

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 74fb15551e83..ea136d79c963 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2752,6 +2752,25 @@ int sev_mem_enc_register_region(struct kvm *kvm,
 
        region->pages = sev_pin_memory(kvm, range->addr, range->size, &region->npages,
                                       FOLL_WRITE | FOLL_LONGTERM);
+
+       /*
+        * On failure, attempt a "short"-term pin for backwards compatibility,
+        * in quotes because this isn't actually a short-term pin.  The kernel
+        * disallows long-term writable pins on file-backed memory as a partial
+        * defense against the fundamental problem that most filesystems don't
+        * play nice with kernel writes via GUP (true short-term pins are much
+        * less likely to be problematic).
+        *
+        * Unfortunately, KVM (incorrectly) used a short-term pin for years,
+        * and so can't *require* a long-term pin.  And for this use case, the
+        * potential filesystem crashes that occur with kernel writes are a
+        * non-issue, as KVM isn't using this pin to access guest memory, the
+        * pin is performed purely to prevent the memory from being migrated.
+        */
+       if (IS_ERR(region->pages))
+               region->pages = sev_pin_memory(kvm, range->addr, range->size,
+                                              &region->npages, FOLL_WRITE);
+
        if (IS_ERR(region->pages)) {
                ret = PTR_ERR(region->pages);
                goto e_free;

> I assume we fail in check_vma_flags()
> 
> 	if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
> 		return -EOPNOTSUPP;
> 
> IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
> 
> How does vfio deal with that? (does it?)
> 
> -- 
> Cheers,
> 
> David

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
  2026-07-01 16:30   ` Sean Christopherson
@ 2026-07-01 16:39     ` David Hildenbrand (Arm)
  2026-07-01 16:56       ` Sean Christopherson
  0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-01 16:39 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
	thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable

On 7/1/26 18:30, Sean Christopherson wrote:
> On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
>> On 7/1/26 16:45, Pankaj Gupta wrote:
>>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
>>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
>>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
>>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
>>> FOLL_WRITE | FOLL_LONGTERM since:
>>>
>>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
>>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
>>>
>>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
>>> the previous behavior.
>>
>> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
> 
> Ya.
> 
>> If it is a longterm pin, it must use FOLL_LONGTERM. :/
> 
> Heh, well, KVM showed that that's not entirely true for many years :-)

What exactly do you mean? KVM MMUs sync through memory notifiers and doesn't
need this.

It's only our "interesting" CoCo code :)

> 
> Assuming we can't solve this some other way, and that there are "real" use cases
> that were broken by adding FOLL_LONGTERM, maybe this as a hack-a-fix?

Well, it's not a driver's decision to make. :P

But, can we actually whitelist virtio-pmem in GUP code somehow?

I mean, it does not suffer from the documented writeback issue, that we wanted
to protect from. We similarly allow shmem and hugetlb there.


-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
  2026-07-01 16:39     ` David Hildenbrand (Arm)
@ 2026-07-01 16:56       ` Sean Christopherson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2026-07-01 16:56 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
	thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable

On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> On 7/1/26 18:30, Sean Christopherson wrote:
> > On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> >> On 7/1/26 16:45, Pankaj Gupta wrote:
> >>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> >>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> >>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> >>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> >>> FOLL_WRITE | FOLL_LONGTERM since:
> >>>
> >>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> >>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
> >>>
> >>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> >>> the previous behavior.
> >>
> >> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
> > 
> > Ya.
> > 
> >> If it is a longterm pin, it must use FOLL_LONGTERM. :/
> > 
> > Heh, well, KVM showed that that's not entirely true for many years :-)
> 
> What exactly do you mean? KVM MMUs sync through memory notifiers and doesn't
> need this.
> 
> It's only our "interesting" CoCo code :)

Yeah, I'm just being cheeky and saying that it's obviously possible to do what
is effectively a long-term pint without specifying FOLL_LONGTERM, i.e. saying
it "must" use FOLL_LONGTERM isn't super duper strictly true.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-01 16:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 14:45 [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration Pankaj Gupta
2026-07-01 14:58 ` sashiko-bot
2026-07-01 16:25 ` David Hildenbrand (Arm)
2026-07-01 16:30   ` Sean Christopherson
2026-07-01 16:39     ` David Hildenbrand (Arm)
2026-07-01 16:56       ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox