linux-kernel - Re: [PATCH] KVM: nVMX: set rflags to specify success in handle

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

Message-ID: <668e0827d62c489cbf52b7bc5d27ba9b@huawei.com>
Date:   Mon, 3 Feb 2020 03:29:19 +0000
From:   linmiaohe <linmiaohe@...wei.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Jim Mattson <jmattson@...gle.com>
CC:     kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] KVM: nVMX: set rflags to specify success in
 handle_invvpid() default case

Vitaly Kuznetsov <vkuznets@...hat.com> writes:
> Sean Christopherson <sean.j.christopherson@...el.com> writes:
>> On Thu, Jan 23, 2020 at 10:22:24AM -0800, Jim Mattson wrote:
>>> On Thu, Jan 23, 2020 at 1:54 AM Paolo Bonzini <pbonzini@...hat.com> wrote:
>>> >
>>> > On 23/01/20 10:45, Vitaly Kuznetsov wrote:
>>> > >>> SDM says that "If an
>>> > >>> unsupported INVVPID type is specified, the instruction fails." 
>>> > >>> and this is similar to INVEPT and I decided to check what 
>>> > >>> handle_invept() does. Well, it does BUG_ON().
>>> > >>>
>>> > >>> Are we doing the right thing in any of these cases?
>>> > >>
>>> > >> Yes, both INVEPT and INVVPID catch this earlier.
>>> > >>
>>> > >> So I'm leaning towards not applying Miaohe's patch.
>>> > >
>>> > > Well, we may at least want to converge on BUG_ON() for both 
>>> > > handle_invvpid()/handle_invept(), there's no need for them to differ.
>>> >
>>> > WARN_ON_ONCE + nested_vmx_failValid would probably be better, if we 
>>> > really want to change this.
>>> >
>>> > Paolo
>>> 
>>> In both cases, something is seriously wrong. The only plausible 
>>> explanations are compiler error or hardware failure. It would be nice 
>>> to handle *all* such failures with a KVM_INTERNAL_ERROR exit to 
>>> userspace. (I'm also thinking of situations like getting a VM-exit 
>> for
>>>> INIT.)
>>
>> Ya.  Vitaly and I had a similar discussion[*].  The idea we tossed 
>> around was to also mark the VM as having encountered a KVM/hardware 
>> bug so that the VM is effectively dead.  That would also allow 
>> gracefully handling bugs that are detected deep in the stack, i.e. 
>> can't simply return 0 to get out to userspace.
>
>Yea, I was thinking about introducing a big hammer which would stop the whole VM as soon as possible to make it easier to debug such situations. Something like (not really tested):
>
Yea, please just ignore my origin patch and do what you want. :)
I'm sorry for reply in such a big day. I'am just backing from a really hard festival. :(