linux-kernel - Re: [PATCH 1/3 v2] KVM: vmx: Enable VMFUNCs

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: <67f46756-7346-8280-7a7e-3f9898d18350@redhat.com>
Date:   Mon, 10 Jul 2017 11:17:33 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     David Hildenbrand <david@...hat.com>, Bandan Das <bsd@...hat.com>,
        kvm@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3 v2] KVM: vmx: Enable VMFUNCs



On 10/07/2017 10:54, David Hildenbrand wrote:
> 
>>  /*
>>   * The exit handlers return 1 if the exit was handled fully and guest execution
>>   * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
>> @@ -7790,6 +7806,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
>> +	[EXIT_REASON_VMFUNC]                  = handle_vmfunc,
>>  	[EXIT_REASON_PREEMPTION_TIMER]	      = handle_preemption_timer,
>>  };
>>  
>> @@ -8111,6 +8128,9 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
>>  	case EXIT_REASON_PML_FULL:
>>  		/* We emulate PML support to L1. */
>>  		return false;
>> +	case EXIT_REASON_VMFUNC:
>> +		/* VM functions are emulated through L2->L0 vmexits. */
>> +		return false;
> 
> This would fit better into the second patch.

It depends on how you reason about it.  I put it here because:

- until this patch, EXIT_REASON_VMFUNC should never be generated.  We
don't even know that it exists.

- after this patch, it should still never be generated in nested
scenarios.  However, if it did because of a bug, we're in a better place
to handle it than L1 (because as far as L1 knows, it should never be
generated).

Perhaps this is an argument in favor of changing the default case of
nested_vmx_exit_handled from true to false.

Paolo

>>  	default:
>>  		return true;
>>  	}
>>
> 
> 
> Looks good to me.
>