linux-kernel - Re: [PATCH] KVM: x86: Deflect unknown MSR accesses to user space

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:   Tue, 28 Jul 2020 10:13:13 -0700
From:   Jim Mattson <jmattson@...gle.com>
To:     Alexander Graf <graf@...zon.com>,
        Aaron Lewis <aaronlewis@...gle.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>, kvm list <kvm@...r.kernel.org>,
        linux-doc@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: x86: Deflect unknown MSR accesses to user space

On Tue, Jul 28, 2020 at 5:41 AM Alexander Graf <graf@...zon.com> wrote:
>
>
>
> On 28.07.20 10:15, Vitaly Kuznetsov wrote:
> >
> > Alexander Graf <graf@...zon.com> writes:
> >
> >> MSRs are weird. Some of them are normal control registers, such as EFER.
> >> Some however are registers that really are model specific, not very
> >> interesting to virtualization workloads, and not performance critical.
> >> Others again are really just windows into package configuration.
> >>
> >> Out of these MSRs, only the first category is necessary to implement in
> >> kernel space. Rarely accessed MSRs, MSRs that should be fine tunes against
> >> certain CPU models and MSRs that contain information on the package level
> >> are much better suited for user space to process. However, over time we have
> >> accumulated a lot of MSRs that are not the first category, but still handled
> >> by in-kernel KVM code.
> >>
> >> This patch adds a generic interface to handle WRMSR and RDMSR from user
> >> space. With this, any future MSR that is part of the latter categories can
> >> be handled in user space.

This sounds similar to Peter Hornyack's RFC from 5 years ago:
https://www.mail-archive.com/kvm@vger.kernel.org/msg124448.html.

> >> Furthermore, it allows us to replace the existing "ignore_msrs" logic with
> >> something that applies per-VM rather than on the full system. That way you
> >> can run productive VMs in parallel to experimental ones where you don't care
> >> about proper MSR handling.
> >>
> >
> > In theory, we can go further: userspace will give KVM the list of MSRs
> > it is interested in. This list may even contain MSRs which are normally
> > handled by KVM, in this case userspace gets an option to mangle KVM's
> > reply (RDMSR) or do something extra (WRMSR). I'm not sure if there is a
> > real need behind this, just an idea.
> >
> > The problem with this approach is: if currently some MSR is not
> > implemented in KVM you will get an exit. When later someone comes with a
> > patch to implement this MSR your userspace handling will immediately get
> > broken so the list of not implemented MSRs effectively becomes an API :-)

Indeed. This is a legitimate concern. At Google, we have experienced
this problem already, using Peter Hornyack's approach. We ended up
commenting out some MSRs from kvm, which is less than ideal.

> Yeah, I'm not quite sure how to do this without bloating the kernel's
> memory footprint too much though.
>
> One option would be to create a shared bitmap with user space. But that
> would need to be sparse and quite big to be able to address all of
> today's possible MSR indexes. From a quick glimpse at Linux's MSR
> defines, there are:
>
>    0x00000000 - 0x00001000 (Intel)
>    0x00001000 - 0x00002000 (VIA)
>    0x40000000 - 0x50000000 (PV)
>    0xc0000000 - 0xc0003000 (AMD)
>    0xc0010000 - 0xc0012000 (AMD)
>    0x80860000 - 0x80870000 (Transmeta)
>
> Another idea would be to turn the logic around and implement an
> allowlist in KVM with all of the MSRs that KVM should handle. In that
> API we could ask for an array of KVM supported MSRs into user space.
> User space could then bounce that array back to KVM to have all in-KVM
> supported MSRs handled. Or it could remove entries that it wants to
> handle on its own.
>
> KVM internally could then save the list as a dense bitmap, translating
> every list entry into its corresponding bit.
>
> While it does feel a bit overengineered, it would solve the problem that
> we're turning in-KVM handled MSRs into an ABI.

It seems unlikely that userspace is going to know what to do with a
large number of MSRs. I suspect that a small enumerated list will
suffice. In fact, +Aaron Lewis is working on upstreaming a local
Google patch set that does just that.