| 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
| ||
|
Date: Thu, 11 Nov 2021 09:14:22 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>
Cc: Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: SEV: Return appropriate error codes if SEV-ES
scratch setup fails
On 11/9/21 4:23 PM, Sean Christopherson wrote:
> Return appropriate error codes if setting up the GHCB scratch area for an
> SEV-ES guest fails. In particular, returning -EINVAL instead of -ENOMEM
> when allocating the kernel buffer could be confusing as userspace would
> likely suspect a guest issue.
Based on previous feedback and to implement the changes to the GHCB
specification, I'm planning on submitting a patch that will return an
error code back to the guest, instead of terminating the guest, if the
scratch area fails to be setup properly. So you could hold off on this
patch if you want.
Thanks,
Tom
>
> Fixes: 8f423a80d299 ("KVM: SVM: Support MMIO for an SEV-ES guest")
> Cc: Tom Lendacky <thomas.lendacky@....com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/x86/kvm/svm/sev.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 3e2769855e51..ea8069c9b5cb 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2299,7 +2299,7 @@ void pre_sev_run(struct vcpu_svm *svm, int cpu)
> }
>
> #define GHCB_SCRATCH_AREA_LIMIT (16ULL * PAGE_SIZE)
> -static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> +static int setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> {
> struct vmcb_control_area *control = &svm->vmcb->control;
> struct ghcb *ghcb = svm->ghcb;
> @@ -2310,14 +2310,14 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> scratch_gpa_beg = ghcb_get_sw_scratch(ghcb);
> if (!scratch_gpa_beg) {
> pr_err("vmgexit: scratch gpa not provided\n");
> - return false;
> + return -EINVAL;
> }
>
> scratch_gpa_end = scratch_gpa_beg + len;
> if (scratch_gpa_end < scratch_gpa_beg) {
> pr_err("vmgexit: scratch length (%#llx) not valid for scratch address (%#llx)\n",
> len, scratch_gpa_beg);
> - return false;
> + return -EINVAL;
> }
>
> if ((scratch_gpa_beg & PAGE_MASK) == control->ghcb_gpa) {
> @@ -2335,7 +2335,7 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> scratch_gpa_end > ghcb_scratch_end) {
> pr_err("vmgexit: scratch area is outside of GHCB shared buffer area (%#llx - %#llx)\n",
> scratch_gpa_beg, scratch_gpa_end);
> - return false;
> + return -EINVAL;
> }
>
> scratch_va = (void *)svm->ghcb;
> @@ -2348,18 +2348,18 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> if (len > GHCB_SCRATCH_AREA_LIMIT) {
> pr_err("vmgexit: scratch area exceeds KVM limits (%#llx requested, %#llx limit)\n",
> len, GHCB_SCRATCH_AREA_LIMIT);
> - return false;
> + return -EINVAL;
> }
> scratch_va = kzalloc(len, GFP_KERNEL_ACCOUNT);
> if (!scratch_va)
> - return false;
> + return -ENOMEM;
>
> if (kvm_read_guest(svm->vcpu.kvm, scratch_gpa_beg, scratch_va, len)) {
> /* Unable to copy scratch area from guest */
> pr_err("vmgexit: kvm_read_guest for scratch area failed\n");
>
> kfree(scratch_va);
> - return false;
> + return -EFAULT;
> }
>
> /*
> @@ -2375,7 +2375,7 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
> svm->ghcb_sa = scratch_va;
> svm->ghcb_sa_len = len;
>
> - return true;
> + return 0;
> }
>
> static void set_ghcb_msr_bits(struct vcpu_svm *svm, u64 value, u64 mask,
> @@ -2514,10 +2514,10 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
> ghcb_set_sw_exit_info_1(ghcb, 0);
> ghcb_set_sw_exit_info_2(ghcb, 0);
>
> - ret = -EINVAL;
> switch (exit_code) {
> case SVM_VMGEXIT_MMIO_READ:
> - if (!setup_vmgexit_scratch(svm, true, control->exit_info_2))
> + ret = setup_vmgexit_scratch(svm, true, control->exit_info_2);
> + if (ret)
> break;
>
> ret = kvm_sev_es_mmio_read(vcpu,
> @@ -2526,7 +2526,8 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
> svm->ghcb_sa);
> break;
> case SVM_VMGEXIT_MMIO_WRITE:
> - if (!setup_vmgexit_scratch(svm, false, control->exit_info_2))
> + ret = setup_vmgexit_scratch(svm, false, control->exit_info_2);
> + if (ret)
> break;
>
> ret = kvm_sev_es_mmio_write(vcpu,
> @@ -2569,6 +2570,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
> vcpu_unimpl(vcpu,
> "vmgexit: unsupported event - exit_info_1=%#llx, exit_info_2=%#llx\n",
> control->exit_info_1, control->exit_info_2);
> + ret = -EINVAL;
> break;
> default:
> ret = svm_invoke_exit_handler(vcpu, exit_code);
> @@ -2579,8 +2581,11 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
>
> int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in)
> {
> - if (!setup_vmgexit_scratch(svm, in, svm->vmcb->control.exit_info_2))
> - return -EINVAL;
> + int r;
> +
> + r = setup_vmgexit_scratch(svm, in, svm->vmcb->control.exit_info_2);
> + if (r)
> + return r;
>
> return kvm_sev_es_string_io(&svm->vcpu, size, port,
> svm->ghcb_sa, svm->ghcb_sa_len / size, in);
>
Powered by blists - more mailing lists