linux-kernel - Re: [PATCH] KVM: SVM: Fix SEV SEND_START session length & SEND_UPDATE_DATA query length after commit 238eca821cee

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: <c579735c-31c3-eed5-576f-b07177231790@redhat.com>
Date:   Tue, 8 Jun 2021 18:22:14 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Ashish Kalra <Ashish.Kalra@....com>
Cc:     seanjc@...gle.com, tglx@...utronix.de, bp@...en8.de,
        mingo@...hat.com, hpa@...or.com, joro@...tes.org,
        Thomas.Lendacky@....com, x86@...nel.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, srutherford@...gle.com,
        brijesh.singh@....com
Subject: Re: [PATCH] KVM: SVM: Fix SEV SEND_START session length &
 SEND_UPDATE_DATA query length after commit 238eca821cee

On 07/06/21 08:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@....com>
> 
> Commit 238eca821cee ("KVM: SVM: Allocate SEV command structures on local stack")
> uses the local stack to allocate the structures used to communicate with the PSP,
> which were earlier being kzalloced. This breaks SEV live migration for
> computing the SEND_START session length and SEND_UPDATE_DATA query length as
> session_len and trans_len and hdr_len fields are not zeroed respectively for
> the above commands before issuing the SEV Firmware API call, hence the
> firmware returns incorrect session length and update data header or trans length.
> 
> Also the SEV Firmware API returns SEV_RET_INVALID_LEN firmware error
> for these length query API calls, and the return value and the
> firmware error needs to be passed to the userspace as it is, so
> need to remove the return check in the KVM code.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
> ---
>   arch/x86/kvm/svm/sev.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5bc887e9a986..e0ce5da97fc2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1103,10 +1103,9 @@ __sev_send_start_query_session_length(struct kvm *kvm, struct kvm_sev_cmd *argp,
>   	struct sev_data_send_start data;
>   	int ret;
>   
> +	memset(&data, 0, sizeof(data));
>   	data.handle = sev->handle;
>   	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_START, &data, &argp->error);
> -	if (ret < 0)
> -		return ret;
>   
>   	params->session_len = data.session_len;
>   	if (copy_to_user((void __user *)(uintptr_t)argp->data, params,
> @@ -1215,10 +1214,9 @@ __sev_send_update_data_query_lengths(struct kvm *kvm, struct kvm_sev_cmd *argp,
>   	struct sev_data_send_update_data data;
>   	int ret;
>   
> +	memset(&data, 0, sizeof(data));
>   	data.handle = sev->handle;
>   	ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, &data, &argp->error);
> -	if (ret < 0)
> -		return ret;
>   
>   	params->hdr_len = data.hdr_len;
>   	params->trans_len = data.trans_len;
> 

Queued, thanks.

Paolo