linux-kernel - Re: [PATCH 2/4] KVM: x86: Harden _regs accesses to guard against buggy input

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]

Date:   Thu, 26 May 2022 17:58:35 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Robert Dinse <nanook@...imo.com>,
        Kees Cook <keescook@...omium.org>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH 2/4] KVM: x86: Harden _regs accesses to guard against
 buggy input

Sean Christopherson <seanjc@...gle.com> writes:

> On Thu, May 26, 2022, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@...gle.com> writes:
>> > ---
>> >  arch/x86/kvm/emulate.c | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> > index 7226a127ccb4..c58366ae4da2 100644
>> > --- a/arch/x86/kvm/emulate.c
>> > +++ b/arch/x86/kvm/emulate.c
>> > @@ -247,6 +247,9 @@ enum x86_transfer_type {
>> >  
>> >  static ulong reg_read(struct x86_emulate_ctxt *ctxt, unsigned nr)
>> >  {
>> > +	if (WARN_ON_ONCE(nr >= 16))
>> > +		nr &= 16 - 1;
>> 
>> As the result of this is unlikely to match the expectation (and I'm
>> unsure what's the expectation here in the first place :-), why not use 
>> KVM_BUG_ON() here instead?
>
> ctxt->vcpu is a 'void *' due to the (IMO futile) separation of the emulator from
> regular KVM.  I.e. this doesn't have access to the 'kvm'.

Well, if we're not emulating something correctly for whatever reason,
killing the VM is likely the right thing to do so I'm going to vote for
abandoning the futility, making ctxt->vcpu 'struct kvm_vcpu *' and doing
KVM_BUG_ON(). (Not necessarily now, the patch looks good to me).

-- 
Vitaly