[PATCH 0/2] KVM: Miscallenous SEV/SNP fixes

[PATCH 0/2] KVM: Miscallenous SEV/SNP fixes

[Atish Patra]

This series addresses a few issues found during code audit of the
KVM SEV/SNP and CCP driver code. The fixes include a incorrect lock state
and incomplete state handling during intra-host migration for SNP VMs.

To: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
To: Borislav Petkov <bp@alien8.de>
To: Dave Hansen <dave.hansen@linux.intel.com>
To: x86@kernel.org
To: H. Peter Anvin <hpa@zytor.com>
To: Tom Lendacky <thomas.lendacky@amd.com>
To: Marc Orr <marcorr@google.com>
To: Peter Gonda <pgonda@google.com>
To: Brijesh Singh <brijesh.singh@amd.com>
To: Youngjae Lee <youngjaelee@meta.com>
To: Ashish Kalra <ashish.kalra@amd.com>
To: Michael Roth <michael.roth@amd.com>
To: John Allen <john.allen@amd.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: clm@meta.com
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: stable@vger.kernel.org

Signed-off-by: Atish Patra <atishp@meta.com>
---
Atish Patra (2):
      KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration
      crypto: ccp: Fix possible deadlock in SEV init failure path

 arch/x86/kvm/svm/sev.c       | 3 ++-
 drivers/crypto/ccp/sev-dev.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
---
base-commit: e7ae89a0c97ce2b68b0983cd01eda67cf373517d
change-id: 20260525-sev_snp_fixes-0b73789c1a91

Best regards,
-- 
Atish Patra <atishp@meta.com>

[PATCH 1/2] KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration

[Atish Patra]

From: Atish Patra <atishp@meta.com>

The intra-host migration feature is not fully implemented for SEV-SNP
VMs which require additional SNP-specific state such as guest_req_mutex,
guest_req_buf, and guest_resp_buf to be transferred or initialized on
the destination.

Reject SNP source VMs in sev_vm_move_enc_context_from() until proper
SNP state transfer is implemented.

Fixes: 0b020f5af092 ("KVM: SEV: Add support for SEV-ES intra host migration")

Reported-by: Chris Mason <clm@meta.com>
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Atish Patra <atishp@meta.com>
---
 arch/x86/kvm/svm/sev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c2126b3c3072..aff6a0cf5bfe 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2142,7 +2142,8 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
 		return ret;
 
 	if (kvm->arch.vm_type != source_kvm->arch.vm_type ||
-	    sev_guest(kvm) || !sev_guest(source_kvm)) {
+	    sev_guest(kvm) || !sev_guest(source_kvm) ||
+	    sev_snp_guest(source_kvm)) {
 		ret = -EINVAL;
 		goto out_unlock;
 	}

-- 
2.53.0-Meta

Re: [PATCH 1/2] KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete protection against SEV-SNP VM state duplication in `sev_vm_copy_enc_context_from`.
--

commit 6c07753da8cad5a8ac0223da06cf2b84411e6fbf
Author: Atish Patra <atishp@meta.com>

    KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration
    
    The intra-host migration feature is not fully implemented for SEV-SNP
    VMs which require additional SNP-specific state such as guest_req_mutex,
    guest_req_buf, and guest_resp_buf to be transferred or initialized on
    the destination.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c2126b3c30724..aff6a0cf5bfe8 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2142,7 +2142,8 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
>  		return ret;
>  
>  	if (kvm->arch.vm_type != source_kvm->arch.vm_type ||
> -	    sev_guest(kvm) || !sev_guest(source_kvm)) {
> +	    sev_guest(kvm) || !sev_guest(source_kvm) ||
> +	    sev_snp_guest(source_kvm)) {

[Severity: High]
This isn't a bug introduced by this patch, but does
sev_vm_copy_enc_context_from() also need a similar check to block SEV-SNP
VMs from VM mirroring?

If userspace creates a mirror of an SNP VM, sev_vm_copy_enc_context_from()
copies its encryption context but fails to copy vmsa_features:

arch/x86/kvm/svm/sev.c:sev_vm_copy_enc_context_from() {
    ...
    mirror_sev->enc_context_owner = source_kvm;
    mirror_sev->active = true;
    mirror_sev->asid = source_sev->asid;
    mirror_sev->fd = source_sev->fd;
    mirror_sev->es_active = source_sev->es_active;
    mirror_sev->need_init = false;
    mirror_sev->handle = source_sev->handle;
    ...
}

Because the mirror's vmsa_features is zero, KVM treats the mirror VM as a
standard SEV-ES VM (since ____sev_snp_guest() will evaluate to false).

When vCPUs are created for the mirror VM, KVM allocates standard SEV-ES
VMSAs and fails to perform SNP-specific initialization, such as registering
the VMSA pages in the RMP via SEV_CMD_SNP_LAUNCH_UPDATE.

When KVM executes VMRUN for the mirror VM's vCPU, the hardware evaluates an
ASID bound to SNP execution using an invalid, non-RMP-registered VMSA page.
Will this trigger an RMP #NPF (Nested Page Fault) on the host hypervisor,
leading to an unhandled hypervisor crash and denial of service?

>  		ret = -EINVAL;
>  		goto out_unlock;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-sev_snp_fixes-v1-0-d67a08151779@meta.com?part=1

Re: [PATCH 1/2] KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration

[Atish Patra]

On 5/28/26 4:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incomplete protection against SEV-SNP VM state duplication in `sev_vm_copy_enc_context_from`.
> --
>
> commit 6c07753da8cad5a8ac0223da06cf2b84411e6fbf
> Author: Atish Patra <atishp@meta.com>
>
>      KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration
>      
>      The intra-host migration feature is not fully implemented for SEV-SNP
>      VMs which require additional SNP-specific state such as guest_req_mutex,
>      guest_req_buf, and guest_resp_buf to be transferred or initialized on
>      the destination.
>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index c2126b3c30724..aff6a0cf5bfe8 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -2142,7 +2142,8 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
>>   		return ret;
>>   
>>   	if (kvm->arch.vm_type != source_kvm->arch.vm_type ||
>> -	    sev_guest(kvm) || !sev_guest(source_kvm)) {
>> +	    sev_guest(kvm) || !sev_guest(source_kvm) ||
>> +	    sev_snp_guest(source_kvm)) {
> [Severity: High]
> This isn't a bug introduced by this patch, but does
> sev_vm_copy_enc_context_from() also need a similar check to block SEV-SNP
> VMs from VM mirroring?
>
> If userspace creates a mirror of an SNP VM, sev_vm_copy_enc_context_from()
> copies its encryption context but fails to copy vmsa_features:
>
> arch/x86/kvm/svm/sev.c:sev_vm_copy_enc_context_from() {
>      ...
>      mirror_sev->enc_context_owner = source_kvm;
>      mirror_sev->active = true;
>      mirror_sev->asid = source_sev->asid;
>      mirror_sev->fd = source_sev->fd;
>      mirror_sev->es_active = source_sev->es_active;
>      mirror_sev->need_init = false;
>      mirror_sev->handle = source_sev->handle;
>      ...
> }
>
> Because the mirror's vmsa_features is zero, KVM treats the mirror VM as a
> standard SEV-ES VM (since ____sev_snp_guest() will evaluate to false).
>
> When vCPUs are created for the mirror VM, KVM allocates standard SEV-ES
> VMSAs and fails to perform SNP-specific initialization, such as registering
> the VMSA pages in the RMP via SEV_CMD_SNP_LAUNCH_UPDATE.
>
> When KVM executes VMRUN for the mirror VM's vCPU, the hardware evaluates an
> ASID bound to SNP execution using an invalid, non-RMP-registered VMSA page.
> Will this trigger an RMP #NPF (Nested Page Fault) on the host hypervisor,
> leading to an unhandled hypervisor crash and denial of service?

Nice catch! I don't think it will lead to hypervisor crash though. As 
per my understanding, the fault on VMSA
page would be sent to user space and just a VM instance will crash which 
is not that critical.

Having said that, we should close the gap for such mistakes from VMM 
side though by rejecting mirroring
of sev-snp VMs. I will add a patch and update the migration kselftest as 
well for sanity checking.

>>   		ret = -EINVAL;
>>   		goto out_unlock;
>>   	}

Re: [PATCH 1/2] KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration

[Sean Christopherson]

On Fri, May 29, 2026, Atish Patra wrote:
> > Because the mirror's vmsa_features is zero, KVM treats the mirror VM as a
> > standard SEV-ES VM (since ____sev_snp_guest() will evaluate to false).
> > 
> > When vCPUs are created for the mirror VM, KVM allocates standard SEV-ES
> > VMSAs and fails to perform SNP-specific initialization, such as registering
> > the VMSA pages in the RMP via SEV_CMD_SNP_LAUNCH_UPDATE.
> > 
> > When KVM executes VMRUN for the mirror VM's vCPU, the hardware evaluates an
> > ASID bound to SNP execution using an invalid, non-RMP-registered VMSA page.
> > Will this trigger an RMP #NPF (Nested Page Fault) on the host hypervisor,
> > leading to an unhandled hypervisor crash and denial of service?
> 
> Nice catch! I don't think it will lead to hypervisor crash though. As per my
> understanding, the fault on VMSA page would be sent to user space and just a
> VM instance will crash which is not that critical.

RMP #PFs on the VMSA will typically crash the host.  In this case, I would
expect VMRUN to fault, which would trigger the BUG_ON() in kvm_spurious_fault().
So assuming the "bad" VM can get far enough along to attempt VMRUN, this is
likely fatal.

> Having said that, we should close the gap for such mistakes from VMM side
> though by rejecting mirroring of sev-snp VMs. I will add a patch and update
> the migration kselftest as well for sanity checking.
> 
> > >   		ret = -EINVAL;
> > >   		goto out_unlock;
> > >   	}

Re: [PATCH 1/2] KVM: SEV: Do not allow SEV-SNP VMs from intra-host migration

[Atish Patra]

On 5/29/26 3:28 PM, Sean Christopherson wrote:
> On Fri, May 29, 2026, Atish Patra wrote:
>>> Because the mirror's vmsa_features is zero, KVM treats the mirror VM as a
>>> standard SEV-ES VM (since ____sev_snp_guest() will evaluate to false).
>>>
>>> When vCPUs are created for the mirror VM, KVM allocates standard SEV-ES
>>> VMSAs and fails to perform SNP-specific initialization, such as registering
>>> the VMSA pages in the RMP via SEV_CMD_SNP_LAUNCH_UPDATE.
>>>
>>> When KVM executes VMRUN for the mirror VM's vCPU, the hardware evaluates an
>>> ASID bound to SNP execution using an invalid, non-RMP-registered VMSA page.
>>> Will this trigger an RMP #NPF (Nested Page Fault) on the host hypervisor,
>>> leading to an unhandled hypervisor crash and denial of service?
>> Nice catch! I don't think it will lead to hypervisor crash though. As per my
>> understanding, the fault on VMSA page would be sent to user space and just a
>> VM instance will crash which is not that critical.
> RMP #PFs on the VMSA will typically crash the host.  In this case, I would
> expect VMRUN to fault, which would trigger the BUG_ON() in kvm_spurious_fault().
> So assuming the "bad" VM can get far enough along to attempt VMRUN, this is
> likely fatal.

Ahh VMRUN would fault it self which would be bad. So no RMP #NPF.
Thanks for the clarification. I will send the v2 with the mirroring fix.

>> Having said that, we should close the gap for such mistakes from VMM side
>> though by rejecting mirroring of sev-snp VMs. I will add a patch and update
>> the migration kselftest as well for sanity checking.
>>
>>>>    		ret = -EINVAL;
>>>>    		goto out_unlock;
>>>>    	}

[PATCH 2/2] crypto: ccp: Fix possible deadlock in SEV init failure path

[Atish Patra]

From: Atish Patra <atishp@meta.com>

__sev_platform_init_handle_init_ex_path() called
rmp_mark_pages_firmware() with locked=false but while the parent
function of init_ex_path already acquired the sev_cmd_mutex.
In case of a rmpupdate failure for any page after the first, the cleanup
path would invoke reclaim pages which would result in a deadlock in
sev_do_cmd.

Pass locked=true to honor the lock status of the parent function.

Fixes: 7364a6fbca45 ("crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled")

Reported-by: Chris Mason <clm@meta.com>
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Atish Patra <atishp@meta.com>
---
 drivers/crypto/ccp/sev-dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index d1e9e0ac63b6..3d4793e8e34b 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1557,7 +1557,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
 		unsigned long npages;
 
 		npages = 1UL << get_order(NV_LENGTH);
-		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, false)) {
+		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, true)) {
 			dev_err(sev->dev, "SEV: INIT_EX NV memory page state change failed.\n");
 			return -ENOMEM;
 		}

-- 
2.53.0-Meta

Re: [PATCH 2/2] crypto: ccp: Fix possible deadlock in SEV init failure path

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Host kernel panics due to RMP violation when reading `sev_init_ex_buffer` in `sev_write_init_ex_file()`.
- [High] Memory allocated for `sev_init_ex_buffer` is leaked and leaves the device in a broken state if initialization fails in `__sev_platform_init_handle_init_ex_path()`.
- [High] Uninitialized kernel memory is allocated and provided to the SEV firmware and disk if the NV initialization file does not exist.
--

commit 405e31473bcf0fe9d84a37db761b7cf915973108
Author: Atish Patra <atishp@meta.com>

crypto: ccp: Fix possible deadlock in SEV init failure path

This commit modifies __sev_platform_init_handle_init_ex_path() to pass
locked=true to rmp_mark_pages_firmware(). This prevents a potential
deadlock on the sev_cmd_mutex when an rmpupdate fails and attempts to
reclaim pages during cleanup.

> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index d1e9e0ac63b60..3d4793e8e34b2 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1557,7 +1557,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>  		unsigned long npages;
>  
>  		npages = 1UL << get_order(NV_LENGTH);

[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of leaking
uninitialized kernel memory to the SEV firmware and disk?

Earlier in this function, the NV buffer is allocated without zeroing the
memory:

	page = alloc_pages(GFP_KERNEL, get_order(NV_LENGTH));
	...
	sev_init_ex_buffer = page_address(page);
	rc = sev_read_init_ex_file();

If the NV initialization file does not exist, sev_read_init_ex_file()
handles the -ENOENT error and returns 0 without reading any data, leaving
the 32KB buffer filled with uninitialized kernel memory. This buffer is
then provided to the SEV firmware.

Later, when state changes occur, sev_write_init_ex_file() writes this memory
back to the host filesystem.

Should alloc_pages() be called with __GFP_ZERO to prevent this?

> -		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, false)) {
> +		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, true)) {
>  			dev_err(sev->dev, "SEV: INIT_EX NV memory page state change failed.\n");
>  			return -ENOMEM;

[Severity: High]
This is a pre-existing issue, but is it possible this error path leaves the
device in a broken state?

If the page state transition fails, the function returns an error without
freeing the buffer or resetting sev_init_ex_buffer to NULL. Because it is a
static global, subsequent initialization attempts will return 0 immediately:

	if (sev_init_ex_buffer)
		return 0;

This bypasses the file reading and the required firmware page state
transition, passing an improperly initialized, host-owned buffer to the
firmware during the INIT_EX command.

Could this permanently break SEV initialization until the system reboots?

[Severity: Critical]
This is also a pre-existing issue, but will reading from sev_init_ex_buffer
trigger a host kernel panic?

Once the sev_init_ex_buffer pages are transitioned to a firmware-owned state
(RMP private) by rmp_mark_pages_firmware(), they are no longer accessible to
the host.

Later, sev_write_init_ex_file() is called to save the NV memory back to disk
and invokes kernel_write() using sev_init_ex_buffer as the buffer.

Since there is no code to reclaim the pages back to a host-accessible state
before this VFS write operation, won't the host CPU's attempt to read from
sev_init_ex_buffer trigger a Page Fault with the RMP bit set?

>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-sev_snp_fixes-v1-0-d67a08151779@meta.com?part=2

Re: [PATCH 2/2] crypto: ccp: Fix possible deadlock in SEV init failure path

[Atish Patra]

On 5/28/26 5:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Host kernel panics due to RMP violation when reading `sev_init_ex_buffer` in `sev_write_init_ex_file()`.
> - [High] Memory allocated for `sev_init_ex_buffer` is leaked and leaves the device in a broken state if initialization fails in `__sev_platform_init_handle_init_ex_path()`.
> - [High] Uninitialized kernel memory is allocated and provided to the SEV firmware and disk if the NV initialization file does not exist.
> --
>
> commit 405e31473bcf0fe9d84a37db761b7cf915973108
> Author: Atish Patra <atishp@meta.com>
>
> crypto: ccp: Fix possible deadlock in SEV init failure path
>
> This commit modifies __sev_platform_init_handle_init_ex_path() to pass
> locked=true to rmp_mark_pages_firmware(). This prevents a potential
> deadlock on the sev_cmd_mutex when an rmpupdate fails and attempts to
> reclaim pages during cleanup.
>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index d1e9e0ac63b60..3d4793e8e34b2 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1557,7 +1557,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>>   		unsigned long npages;
>>   
>>   		npages = 1UL << get_order(NV_LENGTH);
> [Severity: High]
> This isn't a bug introduced by this patch, but is there a risk of leaking
> uninitialized kernel memory to the SEV firmware and disk?
>
> Earlier in this function, the NV buffer is allocated without zeroing the
> memory:
>
> 	page = alloc_pages(GFP_KERNEL, get_order(NV_LENGTH));
> 	...
> 	sev_init_ex_buffer = page_address(page);
> 	rc = sev_read_init_ex_file();
>
> If the NV initialization file does not exist, sev_read_init_ex_file()
> handles the -ENOENT error and returns 0 without reading any data, leaving
> the 32KB buffer filled with uninitialized kernel memory. This buffer is
> then provided to the SEV firmware.
>
> Later, when state changes occur, sev_write_init_ex_file() writes this memory
> back to the host filesystem.
>
> Should alloc_pages() be called with __GFP_ZERO to prevent this?

Valid findings. The sev_init_ex_buffer should be set to NULL and pages 
should be freed in case of failure in sev_read_init_ex_file.


>> -		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, false)) {
>> +		if (rmp_mark_pages_firmware(__pa(sev_init_ex_buffer), npages, true)) {
>>   			dev_err(sev->dev, "SEV: INIT_EX NV memory page state change failed.\n");
>>   			return -ENOMEM;
> [Severity: High]
> This is a pre-existing issue, but is it possible this error path leaves the
> device in a broken state?
>
> If the page state transition fails, the function returns an error without
> freeing the buffer or resetting sev_init_ex_buffer to NULL. Because it is a
> static global, subsequent initialization attempts will return 0 immediately:
>
> 	if (sev_init_ex_buffer)
> 		return 0;
>
> This bypasses the file reading and the required firmware page state
> transition, passing an improperly initialized, host-owned buffer to the
> firmware during the INIT_EX command.
>
> Could this permanently break SEV initialization until the system reboots?

I will add the fix as well for this along with the above one.


> [Severity: Critical]
> This is also a pre-existing issue, but will reading from sev_init_ex_buffer
> trigger a host kernel panic?
>
> Once the sev_init_ex_buffer pages are transitioned to a firmware-owned state
> (RMP private) by rmp_mark_pages_firmware(), they are no longer accessible to
> the host.
>
> Later, sev_write_init_ex_file() is called to save the NV memory back to disk
> and invokes kernel_write() using sev_init_ex_buffer as the buffer.
>
> Since there is no code to reclaim the pages back to a host-accessible state
> before this VFS write operation, won't the host CPU's attempt to read from
> sev_init_ex_buffer trigger a Page Fault with the RMP bit set?

As per my understanding that is not correct. A host cpu write to 
firmware-owner state would result in RMP #PF but read would not raise a 
fault. I confirmed that with loading ccp with init_ex_path and kvm_amd 
as well.

>>   		}