linux-kernel - Re: [PATCH 4/5] KVM: selftests: Explicitly verify KVM doesn't patch hypercall if quirk==off

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: <YyoKhKPf6Bv4X56B@google.com>
Date:   Tue, 20 Sep 2022 18:46:28 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Oliver Upton <oliver.upton@...ux.dev>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Andrew Jones <andrew.jones@...ux.dev>,
        Anup Patel <anup@...infault.org>,
        Atish Patra <atishp@...shpatra.org>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>
Subject: Re: [PATCH 4/5] KVM: selftests: Explicitly verify KVM doesn't patch
 hypercall if quirk==off

On Mon, Sep 19, 2022, Oliver Upton wrote:
> On Thu, Sep 08, 2022 at 11:31:33PM +0000, Sean Christopherson wrote:
> > @@ -75,12 +76,28 @@ static void guest_main(void)
> >  	}
> >  
> >  	/*
> > -	 * The hypercall didn't #UD (guest_ud_handler() signals "done" if a #UD
> > -	 * occurs).  Verify that a #UD is NOT expected and that KVM patched in
> > -	 * the native hypercall.
> > +	 * If the quirk is disabled, verify that guest_ud_handler() "returned"
> > +	 * -EFAULT and that KVM did NOT patch the hypercall.  If the quirk is
> > +	 * enabled, verify that the hypercall succeeded and that KVM patched in
> > +	 * the "right" hypercall.
> >  	 */
> > -	GUEST_ASSERT(!ud_expected);
> > -	GUEST_ASSERT(!memcmp(native_hypercall_insn, hypercall_insn, HYPERCALL_INSN_SIZE));
> > +	if (ud_expected) {
> > +		GUEST_ASSERT(ret == (uint64_t)-EFAULT);
> > +
> > +		/*
> > +		 * Divergence should occur only on the last byte, as the VMCALL
> > +		 * (0F 01 C1) and VMMCALL (0F 01 D9) share the first two bytes.
> > +		 */
> > +		GUEST_ASSERT(!memcmp(native_hypercall_insn, hypercall_insn,
> > +				     HYPERCALL_INSN_SIZE - 1));
> > +		GUEST_ASSERT(memcmp(native_hypercall_insn, hypercall_insn,
> > +				    HYPERCALL_INSN_SIZE));
> 
> Should we just keep the assertions consistent for both cases (patched
> and unpatched)?

Not sure I follow what you're suggesting.  By "consistent" do you mean doing
something like snapshotting hypercall_insn and verifying that it's not changed?