From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752822AbdJNKI6 (ORCPT ); Sat, 14 Oct 2017 06:08:58 -0400 Received: from mail.skyhub.de ([5.9.137.197]:59964 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751728AbdJNKI4 (ORCPT ); Sat, 14 Oct 2017 06:08:56 -0400 Date: Sat, 14 Oct 2017 12:08:47 +0200 From: Borislav Petkov To: Brijesh Singh Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Joerg Roedel , Borislav Petkov , Tom Lendacky Subject: Re: [Part2 PATCH v5 19/31] KVM: SVM: Add support for KVM_SEV_LAUNCH_START command Message-ID: <20171014100847.w3mmelt4qbnbzpca@pd.tnic> References: <20171004131412.13038-1-brijesh.singh@amd.com> <20171004131412.13038-20-brijesh.singh@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20171004131412.13038-20-brijesh.singh@amd.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 04, 2017 at 08:14:00AM -0500, Brijesh Singh wrote: > The KVM_SEV_LAUNCH_START command is used to create a memory encryption > context within the SEV firmware. In order to create the memory encryption > context, the guest owner's should provide the guest's policy, its public > Diffie-Hellman (PDH) key and session information. The command implements > the LAUNCH_START flow defined in SEV spec Section 6.2. Minor cleanups: "The KVM_SEV_LAUNCH_START command is used to create a memory encryption context within the SEV firmware. In order to do so, the guest owner should provide the guest's policy, its public Diffie-Hellman (PDH) key and session information. The command implements the LAUNCH_START flow defined in SEV spec Section 6.2." Also, you can avoid the copy_user_blob() duplication by exporting the version in psp-dev.c. Rest is minor cleanups. --- diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 55a4b7b9945c..a0106f31a5a7 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5713,31 +5713,6 @@ static int sev_issue_cmd(int fd, int id, void *data, int *error) return ret; } -static void *copy_user_blob(u64 __user uaddr, u32 len) -{ - void *data; - - if (!uaddr || !len) - return ERR_PTR(-EINVAL); - - /* verify that blob length does not exceed our limit */ - if (len > SEV_FW_BLOB_MAX_SIZE) - return ERR_PTR(-EINVAL); - - data = kmalloc(len, GFP_KERNEL); - if (!data) - return ERR_PTR(-ENOMEM); - - if (copy_from_user(data, (void __user *)(uintptr_t)uaddr, len)) - goto e_free; - - return data; - -e_free: - kfree(data); - return ERR_PTR(-EFAULT); -} - static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) { struct kvm_sev_info *sev = &kvm->arch.sev_info; @@ -5750,8 +5725,7 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) if (!sev_guest(kvm)) return -ENOTTY; - if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, - sizeof(struct kvm_sev_launch_start))) + if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, sizeof(params))) return -EFAULT; start = kzalloc(sizeof(*start), GFP_KERNEL); @@ -5760,7 +5734,7 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) dh_blob = NULL; if (params.dh_uaddr) { - dh_blob = copy_user_blob(params.dh_uaddr, params.dh_len); + dh_blob = psp_copy_user_blob(params.dh_uaddr, params.dh_len); if (IS_ERR(dh_blob)) { ret = PTR_ERR(dh_blob); goto e_free; @@ -5772,7 +5746,7 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) session_blob = NULL; if (params.session_uaddr) { - dh_blob = copy_user_blob(params.session_uaddr, params.session_len); + dh_blob = psp_copy_user_blob(params.session_uaddr, params.session_len); if (IS_ERR(session_blob)) { ret = PTR_ERR(session_blob); goto e_free_dh; @@ -5797,8 +5771,8 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) /* return handle to userspace */ params.handle = start->handle; - if (copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms, - sizeof(struct kvm_sev_launch_start))) { + ret = copy_to_user((void __user *)(uintptr_t)argp->data, ¶ms, sizeof(params)); + if (ret) { sev_unbind_asid(kvm, start->handle); ret = -EFAULT; goto e_free_session; @@ -5834,10 +5808,9 @@ static int svm_mem_enc_op(struct kvm *kvm, void __user *argp) r = sev_guest_init(kvm, &sev_cmd); break; - case KVM_SEV_LAUNCH_START: { + case KVM_SEV_LAUNCH_START: r = sev_launch_start(kvm, &sev_cmd); break; - } default: break; diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index 331d028f9445..676abc233833 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -377,7 +377,7 @@ static int sev_ioctl_pek_csr(struct sev_issue_cmd *argp) return ret; } -static void *copy_user_blob(u64 __user uaddr, u32 len) +void *psp_copy_user_blob(u64 __user uaddr, u32 len) { void *data; @@ -392,7 +392,7 @@ static void *copy_user_blob(u64 __user uaddr, u32 len) if (!data) return ERR_PTR(-ENOMEM); - if (copy_from_user(data, (void __user *)uaddr, len)) + if (copy_from_user(data, (void __user *)(uintptr_t)uaddr, len)) goto e_free; return data; @@ -401,6 +401,7 @@ static void *copy_user_blob(u64 __user uaddr, u32 len) kfree(data); return ERR_PTR(-EFAULT); } +EXPORT_SYMBOL_GPL(psp_copy_user_blob); static int sev_ioctl_pek_cert_import(struct sev_issue_cmd *argp) { @@ -417,7 +418,7 @@ static int sev_ioctl_pek_cert_import(struct sev_issue_cmd *argp) return -ENOMEM; /* copy PEK certificate blobs from userspace */ - pek_blob = copy_user_blob(input.pek_cert_address, input.pek_cert_len); + pek_blob = psp_copy_user_blob(input.pek_cert_address, input.pek_cert_len); if (IS_ERR(pek_blob)) { ret = PTR_ERR(pek_blob); goto e_free; @@ -427,7 +428,7 @@ static int sev_ioctl_pek_cert_import(struct sev_issue_cmd *argp) data->pek_cert_len = input.pek_cert_len; /* copy PEK certificate blobs from userspace */ - oca_blob = copy_user_blob(input.oca_cert_address, input.oca_cert_len); + oca_blob = psp_copy_user_blob(input.oca_cert_address, input.oca_cert_len); if (IS_ERR(oca_blob)) { ret = PTR_ERR(oca_blob); goto e_free_pek; diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h index 17cd13e2bbe4..1bfa4f3a71cc 100644 --- a/include/linux/psp-sev.h +++ b/include/linux/psp-sev.h @@ -638,6 +638,7 @@ int sev_guest_df_flush(int *error); * -%EIO if the sev returned a non-zero return code */ int sev_guest_decommission(struct sev_data_decommission *data, int *error); +void *psp_copy_user_blob(u64 __user uaddr, u32 len); #else /* !CONFIG_CRYPTO_DEV_SP_PSP */ @@ -667,6 +668,8 @@ sev_issue_cmd_external_user(struct file *filep, return -ENODEV; } +static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_PTR(-EINVAL); } + #endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_SEV_H__ */ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.