linux-kernel - Re: [PATCH] KVM: nVMX: Fix nested bus lock VM exit

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: <YS/BrirERUK4uDaI@google.com>
Date:   Wed, 1 Sep 2021 18:08:46 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Chenyi Qiang <chenyi.qiang@...el.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Xiaoyao Li <xiaoyao.li@...el.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: nVMX: Fix nested bus lock VM exit

On Fri, Aug 27, 2021, Chenyi Qiang wrote:
> Nested bus lock VM exits are not supported yet. If L2 triggers bus lock
> VM exit, it will be directed to L1 VMM, which would cause unexpected
> behavior. Therefore, handle L2's bus lock VM exits in L0 directly.
> 
> Fixes: fe6b6bc802b4 ("KVM: VMX: Enable bus lock VM exit")
> Signed-off-by: Chenyi Qiang <chenyi.qiang@...el.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index bc6327950657..754f53cf0f7a 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5873,6 +5873,8 @@ static bool nested_vmx_l0_wants_exit(struct kvm_vcpu *vcpu,
>  	case EXIT_REASON_VMFUNC:
>  		/* VM functions are emulated through L2->L0 vmexits. */
>  		return true;
> +	case EXIT_REASON_BUS_LOCK:
> +		return true;

Hmm, unless there is zero chance of ever exposing BUS_LOCK_DETECTION to L1, it
might be better to handle this in nested_vmx_l1_wants_exit(), e.g.

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b3f77d18eb5a..793534b7eaba 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6024,6 +6024,8 @@ static bool nested_vmx_l1_wants_exit(struct kvm_vcpu *vcpu,
                        SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE);
        case EXIT_REASON_ENCLS:
                return nested_vmx_exit_handled_encls(vcpu, vmcs12);
+       case EXIT_REASON_BUS_LOCK:
+               return nested_cpu_has2(vmcs12, SECONDARY_EXEC_BUS_LOCK_DETECTION);
        default:
                return true;
        }

It's a rather roundabout way of reaching the same result, but I'd prefer to limit
nested_vmx_l0_wants_exit() to cases where L0 wants to handle the exit regardless
of what L1 wants.  This kinda fits that model, but it's not really that L0 "wants"
the exit, it's that L1 can't want the exit.  Does that make sense?


>  	default:
>  		break;
>  	}
> -- 
> 2.17.1
>