Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes
@ 2026-06-01 23:04 Atish Patra
  2026-06-01 23:04 ` [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs Atish Patra
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Atish Patra @ 2026-06-01 23:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Borislav Petkov, Dave Hansen,
	x86, H. Peter Anvin, Tom Lendacky, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko

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: 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>
---
Changes in v2:
- Added fixes based on the reports by Sashiko. 
- Added a kselftest for validating SNP VM mirroring/migration rejection. 
- Link to v1: https://lore.kernel.org/r/20260528-sev_snp_fixes-v1-0-d67a08151779@meta.com

---
Atish Patra (4):
      KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs
      KVM: selftests: Verify SNP VMs are rejected from migration and mirroring
      crypto: ccp: Fix possible deadlock in SEV init failure path
      crypto: ccp: Fix memory leak in SEV INIT_EX path

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

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


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

* [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs
  2026-06-01 23:04 [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes Atish Patra
@ 2026-06-01 23:04 ` Atish Patra
  2026-06-02 14:38   ` Tom Lendacky
  2026-06-01 23:04 ` [PATCH v2 2/4] KVM: selftests: Verify SNP VMs are rejected from migration and mirroring Atish Patra
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2026-06-01 23:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Borislav Petkov, Dave Hansen,
	x86, H. Peter Anvin, Tom Lendacky, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko

From: Atish Patra <atishp@meta.com>

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

The SNP VM mirroring requires vmsa features to be copied as well otherwise
ASID would be bound to SNP range while VM is detected as a SEV VM.

Reject SNP source VMs in migration/mirroring 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>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Atish Patra <atishp@meta.com>
---
 arch/x86/kvm/svm/sev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c2126b3c3072..e6ad6af128c9 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;
 	}
@@ -2865,6 +2866,7 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd)
 	 * created after SEV/SEV-ES initialization, e.g. to init intercepts.
 	 */
 	if (sev_guest(kvm) || !sev_guest(source_kvm) ||
+	    sev_snp_guest(source_kvm) ||
 	    is_mirroring_enc_context(source_kvm) || kvm->created_vcpus) {
 		ret = -EINVAL;
 		goto e_unlock;

-- 
2.53.0-Meta


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

* [PATCH v2 2/4] KVM: selftests: Verify SNP VMs are rejected from migration and mirroring
  2026-06-01 23:04 [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes Atish Patra
  2026-06-01 23:04 ` [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs Atish Patra
@ 2026-06-01 23:04 ` Atish Patra
  2026-06-01 23:04 ` [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path Atish Patra
  2026-06-01 23:04 ` [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path Atish Patra
  3 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2026-06-01 23:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Borislav Petkov, Dave Hansen,
	x86, H. Peter Anvin, Tom Lendacky, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra

From: Atish Patra <atishp@meta.com>

Migration and mirroring of SEV-SNP VMs are not supported yet.

Add two selftests that verify KVM rejects intra-host migration and
mirroring when the source VM is an SNP VM, so the restriction stays enforced
until proper SNP state transfer is implemented.

Signed-off-by: Atish Patra <atishp@meta.com>
---
 .../testing/selftests/kvm/x86/sev_migrate_tests.c  | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86/sev_migrate_tests.c
index 6b0928e69051..acef6ab26d3d 100644
--- a/tools/testing/selftests/kvm/x86/sev_migrate_tests.c
+++ b/tools/testing/selftests/kvm/x86/sev_migrate_tests.c
@@ -313,6 +313,49 @@ static void test_sev_mirror_parameters(void)
 	kvm_vm_free(vm_no_vcpu);
 }
 
+static void test_sev_snp_migrate_reject(void)
+{
+	struct kvm_vm *src_vm, *dst_vm;
+	int ret;
+
+	src_vm = vm_create_barebones_type(KVM_X86_SNP_VM);
+	snp_vm_init(src_vm);
+	__vm_vcpu_add(src_vm, 0);
+	vm_sev_launch(src_vm, snp_default_policy(), NULL);
+
+	dst_vm = vm_create_barebones_type(KVM_X86_SNP_VM);
+	__vm_vcpu_add(dst_vm, 0);
+
+	ret = __sev_migrate_from(dst_vm, src_vm);
+	TEST_ASSERT(ret == -1 && errno == EINVAL,
+		    "SNP VM migration should be rejected. ret: %d, errno: %d",
+		    ret, errno);
+
+	kvm_vm_free(src_vm);
+	kvm_vm_free(dst_vm);
+}
+
+static void test_sev_snp_mirror_reject(void)
+{
+	struct kvm_vm *src_vm, *dst_vm;
+	int ret;
+
+	src_vm = vm_create_barebones_type(KVM_X86_SNP_VM);
+	snp_vm_init(src_vm);
+	__vm_vcpu_add(src_vm, 0);
+	vm_sev_launch(src_vm, snp_default_policy(), NULL);
+
+	dst_vm = aux_vm_create(false);
+
+	ret = __sev_mirror_create(dst_vm, src_vm);
+	TEST_ASSERT(ret == -1 && errno == EINVAL,
+		    "SNP VM mirroring should be rejected. ret: %d, errno: %d",
+		    ret, errno);
+
+	kvm_vm_free(src_vm);
+	kvm_vm_free(dst_vm);
+}
+
 static void test_sev_move_copy(void)
 {
 	struct kvm_vm *dst_vm, *dst2_vm, *dst3_vm, *sev_vm, *mirror_vm,
@@ -384,12 +427,16 @@ int main(int argc, char *argv[])
 		test_sev_migrate_parameters();
 		if (kvm_has_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM))
 			test_sev_move_copy();
+		if (kvm_cpu_has(X86_FEATURE_SEV_SNP))
+			test_sev_snp_migrate_reject();
 	}
 	if (kvm_has_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) {
 		test_sev_mirror(/* es= */ false);
 		if (have_sev_es)
 			test_sev_mirror(/* es= */ true);
 		test_sev_mirror_parameters();
+		if (kvm_cpu_has(X86_FEATURE_SEV_SNP))
+			test_sev_snp_mirror_reject();
 	}
 	return 0;
 }

-- 
2.53.0-Meta


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

* [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path
  2026-06-01 23:04 [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes Atish Patra
  2026-06-01 23:04 ` [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs Atish Patra
  2026-06-01 23:04 ` [PATCH v2 2/4] KVM: selftests: Verify SNP VMs are rejected from migration and mirroring Atish Patra
@ 2026-06-01 23:04 ` Atish Patra
  2026-06-02 14:43   ` Tom Lendacky
  2026-06-01 23:04 ` [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path Atish Patra
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2026-06-01 23:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Borislav Petkov, Dave Hansen,
	x86, H. Peter Anvin, Tom Lendacky, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, 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


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

* [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path
  2026-06-01 23:04 [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes Atish Patra
                   ` (2 preceding siblings ...)
  2026-06-01 23:04 ` [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path Atish Patra
@ 2026-06-01 23:04 ` Atish Patra
  2026-06-02 14:54   ` Tom Lendacky
  3 siblings, 1 reply; 11+ messages in thread
From: Atish Patra @ 2026-06-01 23:04 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Borislav Petkov, Dave Hansen,
	x86, H. Peter Anvin, Tom Lendacky, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko

From: Atish Patra <atishp@meta.com>

allocated pages in _init_ext_path are never freed and sev_init_ex_buffer
is left pointing at the leaked memory in case of any failures during the
function..

Fix by adding an error path that frees the pages and clears
sev_init_ex_buffer. Make sure we only free the memory if the failure
happens before the conversion. Otherwise, we may end up trying to free
up converted pages in case of reclaim failure. rmp_mark_pages_firmware
failures should be rare enough to avoid more code complexity to track
down which pages were reclaimed/leaked vs which are not.

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

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Atish Patra <atishp@meta.com>
---
 drivers/crypto/ccp/sev-dev.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 3d4793e8e34b..8566f164430b 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1550,7 +1550,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
 
 	rc = sev_read_init_ex_file();
 	if (rc)
-		return rc;
+		goto err_free;
 
 	/* If SEV-SNP is initialized, transition to firmware page. */
 	if (sev->snp_initialized) {
@@ -1559,11 +1559,23 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
 		npages = 1UL << get_order(NV_LENGTH);
 		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;
+			rc = -ENOMEM;
+			/*
+			 * Don't free on conversion failure: the rollback may
+			 * have left pages firmware-owned, and a high-order
+			 * block can't be partially freed.
+			 */
+			goto err_reset;
 		}
 	}
 
 	return 0;
+
+err_free:
+	__free_pages(page, get_order(NV_LENGTH));
+err_reset:
+	sev_init_ex_buffer = NULL;
+	return rc;
 }
 
 static int __sev_platform_init_locked(int *error)

-- 
2.53.0-Meta


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

* Re: [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs
  2026-06-01 23:04 ` [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs Atish Patra
@ 2026-06-02 14:38   ` Tom Lendacky
  2026-06-02 18:44     ` Atish Patra
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Lendacky @ 2026-06-02 14:38 UTC (permalink / raw)
  To: Atish Patra, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko

On 6/1/26 18:04, Atish Patra wrote:
> From: Atish Patra <atishp@meta.com>
> 
> The intra-host migration/mirroring feature is not fully implemented for
> SEV-SNP VMs. The proper migration requires additional SNP-specific
> state such as guest_req_mutex, guest_req_buf, and guest_resp_buf to be
> transferred or initialized on the destination.
> 
> The SNP VM mirroring requires vmsa features to be copied as well otherwise
> ASID would be bound to SNP range while VM is detected as a SEV VM.
> 
> Reject SNP source VMs in migration/mirroring until proper SNP state
> transfer is implemented.
> 
> Fixes: 0b020f5af092 ("KVM: SEV: Add support for SEV-ES intra host migration")

Probably not the correct Fixes: tag. It should the tag that first
introduces SNP hypervisor support.

And adding a comment above the if statements that indicate additional
support is required for SNP, so don't allow it for now, would be nice.

Otherwise, for the actual code...

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> 
> Reported-by: Chris Mason <clm@meta.com>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Atish Patra <atishp@meta.com>
> ---
>  arch/x86/kvm/svm/sev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c2126b3c3072..e6ad6af128c9 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;
>  	}
> @@ -2865,6 +2866,7 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd)
>  	 * created after SEV/SEV-ES initialization, e.g. to init intercepts.
>  	 */
>  	if (sev_guest(kvm) || !sev_guest(source_kvm) ||
> +	    sev_snp_guest(source_kvm) ||
>  	    is_mirroring_enc_context(source_kvm) || kvm->created_vcpus) {
>  		ret = -EINVAL;
>  		goto e_unlock;
> 


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

* Re: [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path
  2026-06-01 23:04 ` [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path Atish Patra
@ 2026-06-02 14:43   ` Tom Lendacky
  2026-06-02 18:46     ` Atish Patra
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Lendacky @ 2026-06-02 14:43 UTC (permalink / raw)
  To: Atish Patra, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra

On 6/1/26 18:04, Atish Patra wrote:
> From: Atish Patra <atishp@meta.com>
> 
> __sev_platform_init_handle_init_ex_path() called

s/called/calls/

> rmp_mark_pages_firmware() with locked=false but while the parent

s/but//

> 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

s/In case/In the case/
s/a rmpupdate/an RMPUPDATE/

> 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>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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;
>  		}
> 


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

* Re: [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path
  2026-06-01 23:04 ` [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path Atish Patra
@ 2026-06-02 14:54   ` Tom Lendacky
  2026-06-02 18:17     ` Atish Patra
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Lendacky @ 2026-06-02 14:54 UTC (permalink / raw)
  To: Atish Patra, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko

On 6/1/26 18:04, Atish Patra wrote:
> From: Atish Patra <atishp@meta.com>
> 
> allocated pages in _init_ext_path are never freed and sev_init_ex_buffer
> is left pointing at the leaked memory in case of any failures during the
> function..
> 
> Fix by adding an error path that frees the pages and clears
> sev_init_ex_buffer. Make sure we only free the memory if the failure
> happens before the conversion. Otherwise, we may end up trying to free
> up converted pages in case of reclaim failure. rmp_mark_pages_firmware
> failures should be rare enough to avoid more code complexity to track
> down which pages were reclaimed/leaked vs which are not.
> 
> Fixes: 7364a6fbca45 ("crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled")
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Atish Patra <atishp@meta.com>

Not sure the goto's are the best, but they do the job - just a personal
preference for me here.

The new comment below is a bit verbose, I would think it is sufficient to
just say something like "Pages can be in an inconsistent state, don't
release them back to the system" or such.

It might be nice in the future if we can identify if the reclaim was
successful and use that for determining whether the pages are safe to
freed... but the failure chance should be practically zero, so I'm not
sure it is worth it.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 3d4793e8e34b..8566f164430b 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1550,7 +1550,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>  
>  	rc = sev_read_init_ex_file();
>  	if (rc)
> -		return rc;
> +		goto err_free;
>  
>  	/* If SEV-SNP is initialized, transition to firmware page. */
>  	if (sev->snp_initialized) {
> @@ -1559,11 +1559,23 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>  		npages = 1UL << get_order(NV_LENGTH);
>  		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;
> +			rc = -ENOMEM;
> +			/*
> +			 * Don't free on conversion failure: the rollback may
> +			 * have left pages firmware-owned, and a high-order
> +			 * block can't be partially freed.
> +			 */
> +			goto err_reset;
>  		}
>  	}
>  
>  	return 0;
> +
> +err_free:
> +	__free_pages(page, get_order(NV_LENGTH));
> +err_reset:
> +	sev_init_ex_buffer = NULL;
> +	return rc;
>  }
>  
>  static int __sev_platform_init_locked(int *error)
> 


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

* Re: [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path
  2026-06-02 14:54   ` Tom Lendacky
@ 2026-06-02 18:17     ` Atish Patra
  0 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2026-06-02 18:17 UTC (permalink / raw)
  To: Tom Lendacky, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko


On 6/2/26 7:54 AM, Tom Lendacky wrote:
> On 6/1/26 18:04, Atish Patra wrote:
>> From: Atish Patra <atishp@meta.com>
>>
>> allocated pages in _init_ext_path are never freed and sev_init_ex_buffer
>> is left pointing at the leaked memory in case of any failures during the
>> function..
>>
>> Fix by adding an error path that frees the pages and clears
>> sev_init_ex_buffer. Make sure we only free the memory if the failure
>> happens before the conversion. Otherwise, we may end up trying to free
>> up converted pages in case of reclaim failure. rmp_mark_pages_firmware
>> failures should be rare enough to avoid more code complexity to track
>> down which pages were reclaimed/leaked vs which are not.
>>
>> Fixes: 7364a6fbca45 ("crypto: ccp: Handle non-volatile INIT_EX data when SNP is enabled")
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Signed-off-by: Atish Patra <atishp@meta.com>
> Not sure the goto's are the best, but they do the job - just a personal
> preference for me here.
>
> The new comment below is a bit verbose, I would think it is sufficient to
> just say something like "Pages can be in an inconsistent state, don't
> release them back to the system" or such.
Sure. I will update the comment.
> It might be nice in the future if we can identify if the reclaim was
> successful and use that for determining whether the pages are safe to
> freed... but the failure chance should be practically zero, so I'm not
> sure it is worth it.

Yes. I had started that path but was not sure if the code churn is worth it.
I can send it as a separate patch and we can take a call if you are 
think it's worth.

>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
>> ---
>>   drivers/crypto/ccp/sev-dev.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index 3d4793e8e34b..8566f164430b 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1550,7 +1550,7 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>>   
>>   	rc = sev_read_init_ex_file();
>>   	if (rc)
>> -		return rc;
>> +		goto err_free;
>>   
>>   	/* If SEV-SNP is initialized, transition to firmware page. */
>>   	if (sev->snp_initialized) {
>> @@ -1559,11 +1559,23 @@ static int __sev_platform_init_handle_init_ex_path(struct sev_device *sev)
>>   		npages = 1UL << get_order(NV_LENGTH);
>>   		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;
>> +			rc = -ENOMEM;
>> +			/*
>> +			 * Don't free on conversion failure: the rollback may
>> +			 * have left pages firmware-owned, and a high-order
>> +			 * block can't be partially freed.
>> +			 */
>> +			goto err_reset;
>>   		}
>>   	}
>>   
>>   	return 0;
>> +
>> +err_free:
>> +	__free_pages(page, get_order(NV_LENGTH));
>> +err_reset:
>> +	sev_init_ex_buffer = NULL;
>> +	return rc;
>>   }
>>   
>>   static int __sev_platform_init_locked(int *error)
>>

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

* Re: [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs
  2026-06-02 14:38   ` Tom Lendacky
@ 2026-06-02 18:44     ` Atish Patra
  0 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2026-06-02 18:44 UTC (permalink / raw)
  To: Tom Lendacky, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra,
	Sashiko


On 6/2/26 7:38 AM, Tom Lendacky wrote:
> On 6/1/26 18:04, Atish Patra wrote:
>> From: Atish Patra <atishp@meta.com>
>>
>> The intra-host migration/mirroring feature is not fully implemented for
>> SEV-SNP VMs. The proper migration requires additional SNP-specific
>> state such as guest_req_mutex, guest_req_buf, and guest_resp_buf to be
>> transferred or initialized on the destination.
>>
>> The SNP VM mirroring requires vmsa features to be copied as well otherwise
>> ASID would be bound to SNP range while VM is detected as a SEV VM.
>>
>> Reject SNP source VMs in migration/mirroring until proper SNP state
>> transfer is implemented.
>>
>> Fixes: 0b020f5af092 ("KVM: SEV: Add support for SEV-ES intra host migration")
> Probably not the correct Fixes: tag. It should the tag that first
> introduces SNP hypervisor support.

Ahh yes. Fixed.

> And adding a comment above the if statements that indicate additional
> support is required for SNP, so don't allow it for now, would be nice.
Added.
> Otherwise, for the actual code...
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
>> Reported-by: Chris Mason <clm@meta.com>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Assisted-by: Claude:claude-opus-4-6
>> Signed-off-by: Atish Patra <atishp@meta.com>
>> ---
>>   arch/x86/kvm/svm/sev.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index c2126b3c3072..e6ad6af128c9 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;
>>   	}
>> @@ -2865,6 +2866,7 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd)
>>   	 * created after SEV/SEV-ES initialization, e.g. to init intercepts.
>>   	 */
>>   	if (sev_guest(kvm) || !sev_guest(source_kvm) ||
>> +	    sev_snp_guest(source_kvm) ||
>>   	    is_mirroring_enc_context(source_kvm) || kvm->created_vcpus) {
>>   		ret = -EINVAL;
>>   		goto e_unlock;
>>

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

* Re: [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path
  2026-06-02 14:43   ` Tom Lendacky
@ 2026-06-02 18:46     ` Atish Patra
  0 siblings, 0 replies; 11+ messages in thread
From: Atish Patra @ 2026-06-02 18:46 UTC (permalink / raw)
  To: Tom Lendacky, Sean Christopherson, Paolo Bonzini, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin, Peter Gonda, Brijesh Singh,
	Youngjae Lee, Ashish Kalra, Michael Roth, John Allen, Herbert Xu
  Cc: clm, kvm, linux-kernel, linux-crypto, stable, Atish Patra


On 6/2/26 7:43 AM, Tom Lendacky wrote:
> On 6/1/26 18:04, Atish Patra wrote:
>> From: Atish Patra <atishp@meta.com>
>>
>> __sev_platform_init_handle_init_ex_path() called
> s/called/calls/
>
>> rmp_mark_pages_firmware() with locked=false but while the parent
> s/but//
>
>> 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
> s/In case/In the case/
> s/a rmpupdate/an RMPUPDATE/
>
>> 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>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
Thanks for the review. Fixed the typos in the commit text.

>> ---
>>   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;
>>   		}
>>

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

end of thread, other threads:[~2026-06-02 18:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 23:04 [PATCH v2 0/4] KVM: Miscellaneous SEV/SNP related fixes Atish Patra
2026-06-01 23:04 ` [PATCH v2 1/4] KVM: SEV: Do not allow intra-host migration/mirroring of SNP VMs Atish Patra
2026-06-02 14:38   ` Tom Lendacky
2026-06-02 18:44     ` Atish Patra
2026-06-01 23:04 ` [PATCH v2 2/4] KVM: selftests: Verify SNP VMs are rejected from migration and mirroring Atish Patra
2026-06-01 23:04 ` [PATCH v2 3/4] crypto: ccp: Fix possible deadlock in SEV init failure path Atish Patra
2026-06-02 14:43   ` Tom Lendacky
2026-06-02 18:46     ` Atish Patra
2026-06-01 23:04 ` [PATCH v2 4/4] crypto: ccp: Fix memory leak in SEV INIT_EX path Atish Patra
2026-06-02 14:54   ` Tom Lendacky
2026-06-02 18:17     ` Atish Patra

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