linux-kernel - Re: [PATCH 3/3] KVM: VMX: Extend VMX's #AC interceptor to handle split lock #AC in guest

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:   Thu, 02 Apr 2020 22:07:07 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     x86@...nel.org, "Kenneth R . Crudup" <kenny@...ix.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Xiaoyao Li <xiaoyao.li@...el.com>,
        Nadav Amit <namit@...are.com>,
        Thomas Hellstrom <thellstrom@...are.com>,
        Tony Luck <tony.luck@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Jessica Yu <jeyu@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] KVM: VMX: Extend VMX's #AC interceptor to handle split lock #AC in guest

Sean,

Sean Christopherson <sean.j.christopherson@...el.com> writes:
> On Thu, Apr 02, 2020 at 07:19:44PM +0200, Thomas Gleixner wrote:
>> Sean Christopherson <sean.j.christopherson@...el.com> writes:
>> > +	case AC_VECTOR:
>> > +		/*
>> > +		 * Reflect #AC to the guest if it's expecting the #AC, i.e. has
>> > +		 * legacy alignment check enabled.  Pre-check host split lock
>> > +		 * turned on to avoid the VMREADs needed to check legacy #AC,
>> > +		 * i.e. reflect the #AC if the only possible source is legacy
>> > +		 * alignment checks.
>> > +		 */
>> > +		if (!boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT) ||
>> 
>> I think the right thing to do here is to make this really independent of
>> that feature, i.e. inject the exception if
>> 
>>  (CPL==3 && CR0.AM && EFLAGS.AC) || (FUTURE && (GUEST_TEST_CTRL & SLD))
>> 
>> iow. when its really clear that the guest asked for it. If there is an
>> actual #AC with SLD disabled and !(CPL==3 && CR0.AM && EFLAGS.AC) then
>> something is badly wrong and the thing should just die. That's why I
>> separated handle_guest_split_lock() and tell about that case.
>
> That puts KVM in a weird spot if/when intercepting #AC is no longer
> necessary, e.g. "if" future CPUs happen to gain a feature that traps into
> the hypervisor (KVM) if a potential near-infinite ucode loop is detected.
>
> The only reason KVM intercepts #AC (before split-lock) is to prevent a
> malicious guest from executing a DoS attack on the host by putting the #AC
> handler in ring 3.  Current CPUs will get stuck in ucode vectoring #AC
> faults more or less indefinitely, e.g. long enough to trigger watchdogs in
> the host.

Which is thankfully well documented in the VMX code and the
corresponding chapter in the SDM. 

> Injecting #AC if and only if KVM is 100% certain the guest wants the #AC
> would lead to divergent behavior if KVM chose to not intercept #AC, e.g.

AFAICT, #AC is not really something which is performance relevant, but I
might obviously be uninformed on that.

Assumed it is not, then there is neither a hard requirement nor a real
incentive to give up on intercepting #AC even when future CPUs have a
fix for the above wreckage.

> some theoretical unknown #AC source would conditionally result in exits to
> userspace depending on whether or not KVM wanted to intercept #AC for
> other reasons.

I'd rather like to know when there is an unknown #AC source instead of
letting the guest silently swallow it.

TBH, the more I learn about this, the more I tend to just give up on
this whole split lock stuff in its current form and wait until HW folks
provide something which is actually usable:

   - Per thread
   - Properly distinguishable from a regular #AC via error code

OTOH, that means I won't be able to use it before retirement. Oh well.

Thanks,

        tglx