linux-kernel - Re: WARNING in kvm_inject_emulated_page

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87pnafdjm2.fsf@vitty.brq.redhat.com>
Date:   Thu, 04 Jun 2020 15:51:49 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        sean.j.christopherson@...el.com
Cc:     syzbot <syzbot+2a7156e11dc199bdbd8a@...kaller.appspotmail.com>,
        bp@...en8.de, hpa@...or.com, jmattson@...gle.com, joro@...tes.org,
        linux-kernel@...r.kernel.org, mingo@...hat.com,
        syzkaller-bugs@...glegroups.com, tglx@...utronix.de,
        wanpengli@...cent.com, x86@...nel.org
Subject: Re: WARNING in kvm_inject_emulated_page_fault

Paolo Bonzini <pbonzini@...hat.com> writes:

> On 04/06/20 12:53, Vitaly Kuznetsov wrote:
>> Exception we're trying to inject comes from
>> 
>>  nested_vmx_get_vmptr()
>>   kvm_read_guest_virt()
>>    kvm_read_guest_virt_helper()
>>      vcpu->arch.walk_mmu->gva_to_gpa()
>> 
>> but it seems it is only set if GVA to GPA convertion fails. In case it
>> doesn't, we can still fail kvm_vcpu_read_guest_page() and return
>> X86EMUL_IO_NEEDED but nested_vmx_get_vmptr() doesn't case what we return
>> and does kvm_inject_emulated_page_fault(). This can happen when VMXON
>> parameter is MMIO, for example.
>> 
>> How do fix this? We can either properly exit to userspace for handling
>> or, if we decide that handling such requests makes little sense, just
>> inject #GP if exception is not set, e.g. 
>> 
>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>> index 9c74a732b08d..a21e2f32f59b 100644
>> --- a/arch/x86/kvm/vmx/nested.c
>> +++ b/arch/x86/kvm/vmx/nested.c
>> @@ -4635,7 +4635,11 @@ static int nested_vmx_get_vmptr(struct kvm_vcpu *vcpu, gpa_t *vmpointer)
>>                 return 1;
>>  
>>         if (kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e)) {
>> -               kvm_inject_emulated_page_fault(vcpu, &e);
>> +               if (e.vector == PF_VECTOR)
>> +                       kvm_inject_emulated_page_fault(vcpu, &e);
>> +               else
>> +                       kvm_inject_gp(vcpu, 0);
>> +
>>                 return 1;
>>         }
>> 
>
> Yes, this is a plausible fix (with a comment explaining that we are 
> taking a shortcut).  Perhaps a better check would be 
>
> 	r = kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e);
> 	if (r != X86EMUL_CONTINUE) {
> 		if (r == X86EMUL_PROPAGATE_FAULT) {
> 			kvm_inject_emulated_page_fault(vcpu, &e);
> 		} else {
> 			/* ... */
> 			kvm_inject_gp(vcpu, 0);
> 		}
> 		return 1;
> 	}
>
> Are you going to send a patch?
>

Sure, will do.

-- 
Vitaly