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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220309052013.GA2915@gao-cwp>
Date:   Wed, 9 Mar 2022 13:21:52 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Zeng Guang <guang.zeng@...el.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,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Tony Luck <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>,
        Kai Huang <kai.huang@...el.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Robert Hu <robert.hu@...el.com>,
        Maxim Levitsky <mlevitsk@...hat.com>
Subject: Re: [PATCH v6 6/9] KVM: x86: lapic: don't allow to change APIC ID
 unconditionally

On Tue, Mar 08, 2022 at 11:04:01PM +0000, Sean Christopherson wrote:
>On Fri, Feb 25, 2022, Zeng Guang wrote:
>> From: Maxim Levitsky <mlevitsk@...hat.com>
>> 
>> No normal guest has any reason to change physical APIC IDs,
>
>I don't think we can reasonably assume this, my analysis in the link (that I just
>realized I deleted from context here) shows it's at least plausible that an existing
>guest could rely on the APIC ID being writable.  And that's just one kernel, who
>know what else is out there, especially given that people use KVM to emulate really
>old stuff, often on really old hardware.

Making xAPIC ID readonly is not only based on your analysis, but also Intel SDM
clearly saying writable xAPIC ID is processor model specific and ***software should
avoid writing to xAPIC ID***.

If writable xAPIC ID support should be retained and is tied to a module param,
live migration would depend on KVM's module params: e.g., migrate a VM with
modified xAPIC ID (apic_id_readonly off on this system) to one with
xapic_id_readonly on would fail, right? Is this failure desired? if not, we need to
have a VM-scope control. e.g., add an inhibitor of APICv (XAPIC_ID_MODIFIED) and
disable APICv forever for this VM if its vCPUs or QEMU modifies xAPIC ID.

>
>Practically speaking, anyone that wants to deploy IPIv is going to have to make
>the switch at some point, but that doesn't help people running legacy crud that
>don't care about IPIv.
>
>I was thinking a module param would be trivial, and it is (see below) if the
>param is off by default.  A module param will also provide a convenient opportunity
>to resolve the loophole reported by Maxim[1][2], though it's a bit funky.

Could you share the links?

>
>Anyways, with an off-by-default module param, we can just do:
>
>	if (!enable_apicv || !cpu_has_vmx_ipiv() || !xapic_id_readonly)
>		enable_ipiv = false;
>
>Forcing userspace to take advantage of IPIv is rather annoying, but it's not the
>end of world.
>
>Having the param on by default is a mess.  Either we break userspace (above), or
>we only kinda break userspace by having it on iff IPIv is on, but then we end up
>with cyclical dependency hell.  E.g. userspace makes xAPIC ID writable and forces
>on IPIv, which one "wins"? And if it's on by default, we can't fix the loophole
>in KVM_SET_LAPIC.

We are fine with having this param off by default.