Re: [PATCH 5/6] KVM: nSVM: always leave the nested state first on KVM_SET_NESTED_STATE

[Sean Christopherson]

On Wed, Jan 06, 2021, Maxim Levitsky wrote:
> This should prevent bad things from happening if the user calls the
> KVM_SET_NESTED_STATE twice.

This doesn't exactly inspire confidence, nor does it provide much help to
readers that don't already know why KVM should "leave nested" before processing
the rest of kvm_state.

> 
> Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> ---
>  arch/x86/kvm/svm/nested.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index c1a3d0e996add..3aa18016832d0 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1154,8 +1154,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>  	if (is_smm(vcpu) && (kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE))
>  		return -EINVAL;
>  
> +	svm_leave_nested(svm);

nVMX sets a really bad example in that it does vmx_leave_nested(), and many
other things, long before it has vetted the incoming state.  That's not the end
of the word as the caller is likely going to exit if this ioctl() fails, but it
would be nice to avoid such behavior with nSVM, especially since it appears to
be trivially easy to do svm_leave_nested() iff the ioctl() will succeed.

> +
>  	if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) {
> -		svm_leave_nested(svm);
>  		svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
>  		return 0;
>  	}
> -- 
> 2.26.2
>