linux-kernel - Re: [PATCH 2/6] KVM: x86: Fix CPUID range check for Centaur and Hypervisor ranges

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 2 Mar 2020 20:58:38 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Jim Mattson <jmattson@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>, kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        Xiaoyao Li <xiaoyao.li@...el.com>
Subject: Re: [PATCH 2/6] KVM: x86: Fix CPUID range check for Centaur and
 Hypervisor ranges

On Mon, Mar 02, 2020 at 08:25:31PM -0800, Jim Mattson wrote:
> On Mon, Mar 2, 2020 at 7:25 PM Jim Mattson <jmattson@...gle.com> wrote:
> >
> > On Mon, Mar 2, 2020 at 11:57 AM Sean Christopherson
> > <sean.j.christopherson@...el.com> wrote:
> >
> > > The bad behavior can be visually confirmed by dumping CPUID output in
> > > the guest when running Qemu with a stable TSC, as Qemu extends the limit
> > > of range 0x40000000 to 0x40000010 to advertise VMware's cpuid_freq,
> > > without defining zeroed entries for 0x40000002 - 0x4000000f.
> >
> > I think it could be reasonably argued that this is a userspace bug.
> > Clearly, when userspace explicitly supplies the results for a leaf,
> > those results override the default CPUID values for that leaf. But I
> > haven't seen it documented anywhere that leaves *not* explicitly
> > supplied by userspace will override the default CPUID values, just
> > because they happen to appear in some magic range.
> 
> In fact, the more I think about it, the original change is correct, at
> least in this regard. Your "fix" introduces undocumented and
> unfathomable behavior.

Heh, the takeaway from this is that whatever we decide on needs to be
documented somewhere :-)

I wouldn't say it's unfathomable, conceptually it seems like the intent
of the hypervisor range was to mimic the basic and extended ranges.  The
whole thing is arbitrary behavior.  Of course if Intel CPUs would just
return 0s on undefined leafs it would be a lot less arbitrary :-)
 
Anyways, I don't have a strong opinion on whether this patch stays or goes.