linux-kernel - Re: [PATCH v2 05/11] KVM: x86: emulator: update the emulation mode after CR0 write

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: <532c71cbca049004bd6860508fdc056ae118ab1f.camel@redhat.com>
Date:   Thu, 21 Jul 2022 14:53:43 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, x86@...nel.org,
        Kees Cook <keescook@...omium.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>,
        Borislav Petkov <bp@...en8.de>, Joerg Roedel <joro@...tes.org>,
        Ingo Molnar <mingo@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>
Subject: Re: [PATCH v2 05/11] KVM: x86: emulator: update the emulation mode
 after CR0 write

On Wed, 2022-07-20 at 23:50 +0000, Sean Christopherson wrote:
> On Tue, Jun 21, 2022, Maxim Levitsky wrote:
> > CR0.PE toggles real/protected mode, thus its update
> > should update the emulation mode.
> > 
> > This is likely a benign bug because there is no writeback
> > of state, other than the RIP increment, and when toggling
> > CR0.PE, the CPU has to execute code from a very low memory address.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> > ---
> >  arch/x86/kvm/emulate.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > index 6f4632babc4cd8..002687d17f9364 100644
> > --- a/arch/x86/kvm/emulate.c
> > +++ b/arch/x86/kvm/emulate.c
> > @@ -3659,11 +3659,22 @@ static int em_movbe(struct x86_emulate_ctxt *ctxt)
> >  
> >  static int em_cr_write(struct x86_emulate_ctxt *ctxt)
> >  {
> > -	if (ctxt->ops->set_cr(ctxt, ctxt->modrm_reg, ctxt->src.val))
> > +	int cr_num = ctxt->modrm_reg;
> > +	int r;
> > +
> > +	if (ctxt->ops->set_cr(ctxt, cr_num, ctxt->src.val))
> >  		return emulate_gp(ctxt, 0);
> >  
> >  	/* Disable writeback. */
> >  	ctxt->dst.type = OP_NONE;
> > +
> > +	if (cr_num == 0) {
> > +		/* CR0 write might have updated CR0.PE */
> 
> Or toggled CR0.PG.  

I thought about it but paging actually does not affect the CPU mode.

E.g if you are in protected mode, instructions execute the same regardless
if you have paging or not.

(There are probably some exceptions but you understand what I mean).

Best regards,
	Maxim Levitsky

> It's probably also worth noting that ->set_cr() handles side
> effects to other registers, e.g. the lack of an EFER.LMA update makes this look
> suspicious at first glance.

> 
> > +		r = update_emulation_mode(ctxt);
> > +		if (r != X86EMUL_CONTINUE)
> > +			return r;
> > +	}
> > +
> >  	return X86EMUL_CONTINUE;
> >  }
> >  
> > -- 
> > 2.26.3
> >