Re: [BUG] arch/x86/kvm/vmx/vmx_onhyperv.h:109:36: error: dereference of NULL ‘0’

Vitaly Kuznetsov
2024-08-14T15:38:44Z

Sean Christopherson <seanjc@google.com> writes:

> On Wed, Aug 14, 2024, Vitaly Kuznetsov wrote:
>> What I meant is something along these lines (untested):
>>
>> diff --git a/arch/x86/kvm/vmx/vmx_onhyperv.h b/arch/x86/kvm/vmx/vmx_onhyperv.h
>> index eb48153bfd73..e2d8c67d0cad 100644
>> --- a/arch/x86/kvm/vmx/vmx_onhyperv.h
>> +++ b/arch/x86/kvm/vmx/vmx_onhyperv.h
>> @@ -104,6 +104,14 @@ static inline void evmcs_load(u64 phys_addr)
>> struct hv_vp_assist_page *vp_ap =
>> hv_get_vp_assist_page(smp_processor_id());
>>
>> + /*
>> + * When enabling eVMCS, KVM verifies that every CPU has a valid hv_vp_assist_page()
>> + * and aborts enabling the feature otherwise. CPU onlining path is also checked in
>> + * vmx_hardware_enable(). With this, it is impossible to reach here with vp_ap == NULL
>> + * but compilers may still complain.
>> + */
>> + BUG_ON(!vp_ap);
>
> A full BUG_ON() is overkill, and easily avoided. If we want to add a sanity
> check here and do more than just WARN, then it's easy enough to plumb in @vcpu
> and make this a KVM_BUG_ON() so that the VM dies, i.e. so that KVM doesn't risk
> corrupting the guest somehow.
>

I'm still acting under the impression this is an absolutely impossible
situation :-)

AFAICS, we only call evmcs_load() from vmcs_load() but this one doesn't
have @vcpu/@kvm either and I wasn't sure it's worth the effort to do the
plumbing (or am I missing an easy way to go back from @vmcs to
@vcpu?). On the other hand, vmcs_load() should not be called that ofter
so if we prefer to have @vcpu there for some other reason -- why not.

--
Vitaly