linux-kernel - Re: [RFC PATCH v3 03/19] KVM: x86: SVM: remove avic's broken code that updated APIC ID

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: <d1df82a17e6e89ab58d5f6aa911ee2d532efee31.camel@redhat.com>
Date:   Sun, 22 May 2022 12:01:55 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, Wanpeng Li <wanpengli@...cent.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Zhenyu Wang <zhenyuw@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Ingo Molnar <mingo@...hat.com>,
        David Airlie <airlied@...ux.ie>,
        Thomas Gleixner <tglx@...utronix.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        intel-gfx@...ts.freedesktop.org, Daniel Vetter <daniel@...ll.ch>,
        Borislav Petkov <bp@...en8.de>, Joerg Roedel <joro@...tes.org>,
        linux-kernel@...r.kernel.org, Jim Mattson <jmattson@...gle.com>,
        Zhi Wang <zhi.a.wang@...el.com>,
        Brijesh Singh <brijesh.singh@....com>,
        "H. Peter Anvin" <hpa@...or.com>,
        intel-gvt-dev@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [RFC PATCH v3 03/19] KVM: x86: SVM: remove avic's broken code
 that updated APIC ID

On Thu, 2022-05-19 at 16:10 +0000, Sean Christopherson wrote:
> On Wed, Apr 27, 2022, Maxim Levitsky wrote:
> > AVIC is now inhibited if the guest changes apic id, thus remove
> > that broken code.
> 
> Can you explicitly call out what's broken?  Just something short on the code not
> handling the scenario where APIC ID is changed back to vcpu_id to help future
> archaeologists.  I forget if there are other bugs...
> 


Well, the avic_handle_apic_id_update is called each time the AVIC is uninhibited,
because while it is inhibited, the AVIC code doesn't track changes to APIC ID and such.

Also there are many ways it is broken for example

1. a CPU can't move its APIC ID to a free slot due to (!new) check

2. If APIC ID is moved to a used slot, then the CPU that used that overwritten
slot can't correctly move it, since its now not its slot, not to mention races.


BTW, if you see a value in it, I can fix this code instead - a lock + going over all the apic ids,
should be quite easy to implement. In case of two vCPUs using the same APIC ID,
I can write non present entry to the table, so none will be able to be addressed,
hoping that the situation is only temporary.

Same can be done for IPIv.

Best regards,
	Maxim Levitsky