linux-kernel - Re: [PATCH 1/5] KVM: SVM: Decrypt SEV VMSA in dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bcd4aa69-6b94-14ba-fc3d-d15e77b0c020@amd.com>
Date: Mon, 14 Apr 2025 12:20:48 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
 Paolo Bonzini <pbonzini@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, Michael Roth <michael.roth@....com>
Subject: Re: [PATCH 1/5] KVM: SVM: Decrypt SEV VMSA in dump_vmcb() if
 debugging is enabled

On 4/14/25 11:11, Tom Lendacky wrote:
> On 4/10/25 18:14, Sean Christopherson wrote:
>> On Mon, Mar 24, 2025, Tom Lendacky wrote:
>>> On 3/20/25 08:26, Tom Lendacky wrote:
>>>> An SEV-ES/SEV-SNP VM save area (VMSA) can be decrypted if the guest
>>>> policy allows debugging. Update the dump_vmcb() routine to output
>>>> some of the SEV VMSA contents if possible. This can be useful for
>>>> debug purposes.
>>>>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>>>> ---
>>>>  arch/x86/kvm/svm/sev.c | 98 ++++++++++++++++++++++++++++++++++++++++++
>>>>  arch/x86/kvm/svm/svm.c | 13 ++++++
>>>>  arch/x86/kvm/svm/svm.h | 11 +++++
>>>>  3 files changed, 122 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>>> index 661108d65ee7..6e3f5042d9ce 100644
>>>> --- a/arch/x86/kvm/svm/sev.c
>>>> +++ b/arch/x86/kvm/svm/sev.c
>>>
>>>> +
>>>> +	if (sev_snp_guest(vcpu->kvm)) {
>>>> +		struct sev_data_snp_dbg dbg = {0};
>>>> +
>>>> +		vmsa = snp_alloc_firmware_page(__GFP_ZERO);
>>>> +		if (!vmsa)
>>>> +			return NULL;
>>>> +
>>>> +		dbg.gctx_paddr = __psp_pa(sev->snp_context);
>>>> +		dbg.src_addr = svm->vmcb->control.vmsa_pa;
>>>> +		dbg.dst_addr = __psp_pa(vmsa);
>>>> +
>>>> +		ret = sev_issue_cmd(vcpu->kvm, SEV_CMD_SNP_DBG_DECRYPT, &dbg, &error);
>>>
>>> This can also be sev_do_cmd() where the file descriptor isn't checked.
>>> Since it isn't really a user initiated call, that might be desirable since
>>> this could also be useful for debugging during guest destruction (when the
>>> file descriptor has already been closed) for VMSAs that haven't exited
>>> with an INVALID exit code.
>>>
>>> Just an FYI, I can change this call and the one below to sev_do_cmd() if
>>> agreed upon.
>>
>> Works for me.  Want to provide a delta patch?  I can fixup when applying.
> 
> Will do.

Here's the diff on top:

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6e3f5042d9ce..4e9ab172e3f0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -5020,7 +5020,7 @@ struct vmcb_save_area *sev_decrypt_vmsa(struct kvm_vcpu *vcpu)
 		dbg.src_addr = svm->vmcb->control.vmsa_pa;
 		dbg.dst_addr = __psp_pa(vmsa);
 
-		ret = sev_issue_cmd(vcpu->kvm, SEV_CMD_SNP_DBG_DECRYPT, &dbg, &error);
+		ret = sev_do_cmd(SEV_CMD_SNP_DBG_DECRYPT, &dbg, &error);
 
 		/*
 		 * Return the target page to a hypervisor page no matter what.
@@ -5052,7 +5052,7 @@ struct vmcb_save_area *sev_decrypt_vmsa(struct kvm_vcpu *vcpu)
 		dbg.dst_addr = __psp_pa(vmsa);
 		dbg.len = PAGE_SIZE;
 
-		ret = sev_issue_cmd(vcpu->kvm, SEV_CMD_DBG_DECRYPT, &dbg, &error);
+		ret = sev_do_cmd(SEV_CMD_DBG_DECRYPT, &dbg, &error);
 		if (ret) {
 			pr_err("SEV: SEV_CMD_DBG_DECRYPT failed ret=%d, fw_error=%d (0x%x)\n",
 			       ret, error, error);

> 
> Thanks,
> Tom