lkml.org 
[lkml]   [2020]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 29/61] KVM: x86: Add Kconfig-controlled auditing of reverse CPUID lookups
    On Mon, Feb 24, 2020 at 02:54:38PM +0100, Vitaly Kuznetsov wrote:
    > Sean Christopherson <sean.j.christopherson@intel.com> writes:
    >
    > > Add WARNs in the low level __cpuid_entry_get_reg() to assert that the
    > > function and index of the CPUID entry and reverse CPUID entry match.
    > > Wrap the WARNs in a new Kconfig, KVM_CPUID_AUDIT, as the checks add
    > > almost no value in a production environment, i.e. will only detect
    > > blatant KVM bugs and fatal hardware errors. Add a Kconfig instead of
    > > simply wrapping the WARNs with an off-by-default #ifdef so that syzbot
    > > and other automated testing can enable the auditing.
    > >
    > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
    > > ---
    > > arch/x86/kvm/Kconfig | 10 ++++++++++
    > > arch/x86/kvm/cpuid.h | 5 +++++
    > > 2 files changed, 15 insertions(+)
    > >
    > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
    > > index 840e12583b85..bbbc3258358e 100644
    > > --- a/arch/x86/kvm/Kconfig
    > > +++ b/arch/x86/kvm/Kconfig
    > > @@ -96,6 +96,16 @@ config KVM_MMU_AUDIT
    > > This option adds a R/W kVM module parameter 'mmu_audit', which allows
    > > auditing of KVM MMU events at runtime.
    > >
    > > +config KVM_CPUID_AUDIT
    > > + bool "Audit KVM reverse CPUID lookups"
    > > + depends on KVM
    > > + help
    > > + This option enables runtime checking of reverse CPUID lookups in KVM
    > > + to verify the function and index of the referenced X86_FEATURE_* match
    > > + the function and index of the CPUID entry being accessed.
    > > +
    > > + If unsure, say N.
    > > +
    > > # OK, it's a little counter-intuitive to do this, but it puts it neatly under
    > > # the virtualization menu.
    > > source "drivers/vhost/Kconfig"
    > > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
    > > index 51f19eade5a0..41ff94a7d3e0 100644
    > > --- a/arch/x86/kvm/cpuid.h
    > > +++ b/arch/x86/kvm/cpuid.h
    > > @@ -98,6 +98,11 @@ static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
    > > static __always_inline u32 *__cpuid_entry_get_reg(struct kvm_cpuid_entry2 *entry,
    > > const struct cpuid_reg *cpuid)
    > > {
    > > +#ifdef CONFIG_KVM_CPUID_AUDIT
    > > + WARN_ON_ONCE(entry->function != cpuid->function);
    > > + WARN_ON_ONCE(entry->index != cpuid->index);
    > > +#endif
    > > +
    > > switch (cpuid->reg) {
    > > case CPUID_EAX:
    > > return &entry->eax;
    >
    > Honestly, I was thinking we should BUG_ON() and even in production builds
    > but not everyone around is so rebellious I guess, so

    LOL. It's a waste of cycles for something that will "never" be hit, i.e.
    we _really_ dropped the ball if a bug of this natures makes it into a
    kernel release.

    > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
    >
    > --
    > Vitaly
    >

    \
     
     \ /
      Last update: 2020-02-24 23:46    [W:3.265 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site