linux-kernel - Re: [PATCH RFC v8 34/56] KVM: SVM: Add KVM_SEV_SNP_LAUNCH

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <629c89dd-4892-28c7-74ac-b3fcacb1550d@amd.com>
Date:   Wed, 26 Apr 2023 13:02:46 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Sabin Rapan <sabrapan@...zon.com>,
        Michael Roth <michael.roth@....com>, kvm@...r.kernel.org
Cc:     linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, x86@...nel.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
        jroedel@...e.de, hpa@...or.com, ardb@...nel.org,
        pbonzini@...hat.com, seanjc@...gle.com, vkuznets@...hat.com,
        jmattson@...gle.com, luto@...nel.org, dave.hansen@...ux.intel.com,
        slp@...hat.com, pgonda@...gle.com, peterz@...radead.org,
        srinivas.pandruvada@...ux.intel.com, rientjes@...gle.com,
        dovmurik@...ux.ibm.com, tobin@....com, bp@...en8.de,
        vbabka@...e.cz, kirill@...temov.name, ak@...ux.intel.com,
        tony.luck@...el.com, marcorr@...gle.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, alpergun@...gle.com,
        dgilbert@...hat.com, jarkko@...nel.org, ashish.kalra@....com,
        nikunj.dadhania@....com, Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH RFC v8 34/56] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_START
 command

On 4/26/23 12:06, Sabin Rapan wrote:
> On 20.02.2023 20:38, Michael Roth wrote:
>> From: Brijesh Singh <brijesh.singh@....com>
>>
>>   
>> +static int snp_decommission_context(struct kvm *kvm)
>> +{
>> +	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
>> +	struct sev_data_snp_addr data = {};
>> +	int ret;
>> +
>> +	/* If context is not created then do nothing */
>> +	if (!sev->snp_context)
>> +		return 0;
>> +
>> +	data.gctx_paddr = __sme_pa(sev->snp_context);
>> +	ret = sev_do_cmd(SEV_CMD_SNP_DECOMMISSION, &data, NULL);
>> +	if (WARN_ONCE(ret, "failed to release guest context"))
>> +		return ret;
>> +
>> +	/* free the context page now */
>> +	snp_free_firmware_page(sev->snp_context);
>> +	sev->snp_context = NULL;
>> +
>> +	return 0;
>> +}
>> +
> 
> Even though it's not documented, SNP_DECOMMISSION seems to clear the
> WBINVD indicator just like DEACTIVATE does for SEV.
> Won't ASID recycling race with SNP_DECOMMISSION if the latter isn't
> guarded with sev_deactivate_lock?

Good catch, yes, this needs to use the sev_deactivate_lock around the 
DECOMMISSION command.

Thanks,
Tom

> 
> 
>>   void sev_vm_destroy(struct kvm *kvm)
>>   {
>>   	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
>> @@ -2333,7 +2440,15 @@ void sev_vm_destroy(struct kvm *kvm)
>>   		}
>>   	}
>>   
>> -	sev_unbind_asid(kvm, sev->handle);
>> +	if (sev_snp_guest(kvm)) {
>> +		if (snp_decommission_context(kvm)) {
>> +			WARN_ONCE(1, "Failed to free SNP guest context, leaking asid!\n");
>> +			return;
>> +		}
>> +	} else {
>> +		sev_unbind_asid(kvm, sev->handle);
>> +	}
>> +
>>   	sev_asid_free(sev);
>>   }
>>   
> 
> 
> 
> Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.