linux-kernel - Re: [PATCH v6 6/9] KVM: x86: lapic: don't allow to change APIC ID unconditionally

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: <c903e82ed2a1e98f66910c35b5aabdcf56e08e72.camel@redhat.com>
Date:   Tue, 15 Mar 2022 17:30:32 +0200
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Chao Gao <chao.gao@...el.com>
Cc:     Zeng Guang <guang.zeng@...el.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Kim Phillips <kim.phillips@....com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Jethro Beekman <jethro@...tanix.com>,
        "Huang, Kai" <kai.huang@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Hu, Robert" <robert.hu@...el.com>
Subject: Re: [PATCH v6 6/9] KVM: x86: lapic: don't allow to change APIC ID
 unconditionally

On Tue, 2022-03-15 at 23:10 +0800, Chao Gao wrote:
> On Sun, Mar 13, 2022 at 05:09:08PM +0200, Maxim Levitsky wrote:
> > > > > This won't work with nested AVIC - we can't just inhibit a nested guest using its own AVIC,
> > > > > because migration happens.
> > > > 
> > > > I mean because host decided to change its apic id, which it can in theory do any time,
> > > > even after the nested guest has started. Seriously, the only reason guest has to change apic id,
> > > > is to try to exploit some security hole.
> > > 
> > > Hi
> > > 
> > > Thanks for the information.  
> > > 
> > > IIUC, you mean KVM applies APICv inhibition only to L1 VM, leaving APICv
> > > enabled for L2 VM. Shouldn't KVM disable APICv for L2 VM in this case?
> > > It looks like a generic issue in dynamically toggling APICv scheme,
> > > e.g., qemu can set KVM_GUESTDBG_BLOCKIRQ after nested guest has started.
> > > 
> > 
> > That is the problem - you can't disable it for L2, unless you are willing to emulate it in software.
> > Or in other words, when nested guest uses a hardware feature, you can't at some point say to it:
> > sorry buddy - hardware feature disappeared.
> 
> Hi Maxim,
> 
> I may miss something. When reading Sean's APICv inhibition cleanups, I
> find AVIC is disabled for L1 when nested is enabled (SVM is advertised
> to L1). Then, I think the new inhibition introduced for changed xAPIC ID
> shouldn't be a problem for L2 VM. Or, you plan to remove
> APICV_INHIBIT_REASON_NESTED and expose AVIC to L1?

Yep, I  have a patch for this ( which I hope to be accepted really soon
(KVM: x86: SVM: allow AVIC to co-exist with a nested guest running)
 
I also implemented working support for nested AVIC, which includes support for IPI without vm exits
between L2's vCPUs. I had sent an RFC for that.
 
With all patches applied both L1 and L2 switch hands on AVIC, L1's avic is inhibited
(only locally) on the vCPU which runs nested, and while it runs nested, L2 uses AVIC
to target other vCPUs which also run nested.
 
I and Paolo talked about this, and we reached a very promising conclusion.

I will add new KVM cap, say KVM_CAP_READ_ONLY_APIC, which userspace will set
prior to creating a vCPU, and which will make APIC ID fully readonly when set.
 
As a bonus, if you don't object, I will also make this cap, make APIC base read-only,
since this feature is also broken in kvm, optional in x86 spec, and not really
used by guests just like writable apic id.

I hope to have patches in day or two for this.
 
When this cap is not set, it is fair to disable both IPIv, my nested AVIC,
or even better inhibit AVIC completely, including any nested support.
 
Best regards,
	Maxim Levitsky

> 
> svm_vcpu_after_set_cpuid:
>                 /*
>                  * Currently, AVIC does not work with nested virtualization.
>                  * So, we disable AVIC when cpuid for SVM is set in the L1 guest.
>                  */
>                 if (nested && guest_cpuid_has(vcpu, X86_FEATURE_SVM))
>                         kvm_request_apicv_update(vcpu->kvm, false,
>                                                  APICV_INHIBIT_REASON_NESTED);
>