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

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

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.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@amd.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;
-- 
2.17.1

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

[Paolo Bonzini]

On 07/06/21 08:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.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@amd.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